linux-kernel - Re: [PATCH] of: do not leak console options

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170827080148.GB581@tigerII.localdomain>
Date:   Sun, 27 Aug 2017 17:01:48 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Grant Likely <grant.likely@...retlab.ca>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] of: do not leak console options

On (08/27/17 16:19), Sergey Senozhatsky wrote:
[..]
> int uart_parse_earlycon(char *p, unsigned char *iotype, resource_size_t *addr,
> 			char **options)
> {
> 	if (strncmp(p, "mmio,", 5) == 0) {
> 		*iotype = UPIO_MEM;
> 		p += 5;
> 	} else if (strncmp(p, "mmio16,", 7) == 0) {
> 		*iotype = UPIO_MEM16;
> 		p += 7;
> 	} else if (strncmp(p, "mmio32,", 7) == 0) {
> 		*iotype = UPIO_MEM32;
> 		p += 7;
> 	} else if (strncmp(p, "mmio32be,", 9) == 0) {
> 		*iotype = UPIO_MEM32BE;
> 		p += 9;
> 	} else if (strncmp(p, "mmio32native,", 13) == 0) {
> 		*iotype = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) ?
> 			UPIO_MEM32BE : UPIO_MEM32;
> 		p += 13;
> 	} else if (strncmp(p, "io,", 3) == 0) {
> 		*iotype = UPIO_PORT;
> 		p += 3;
> 	} else if (strncmp(p, "0x", 2) == 0) {
> 		*iotype = UPIO_MEM;
> 	} else {
> 		return -EINVAL;
> 	}
> 
> 	/*
> 	 * Before you replace it with kstrtoull(), think about options separator
> 	 * (',') it will not tolerate
> 	 */
> 	*addr = simple_strtoull(p, NULL, 0);
> 	p = strchr(p, ',');
> 	if (p)
> 		p++;
> 
> 	*options = p;
> 	return 0;
> }
> 
> that's just one example I found after a very quick grepping. may be
> there are other control paths that can change ->options.

well, obviously, it doesn't change the options per se, but it sort of
demonstrates that _probably_ somewhere ->options could be changed. on
the other hand, that ->options = ->options + X makes it rather hard
to kfree() the ->options. well, we never kfree() the ->options anyway,
as far as I can tell...

	-ss