linux-kernel - Re: [PATCH]early: Fix possible overlapping data buffer

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] [day] [month] [year] [list]

Date:	Wed, 16 Mar 2011 10:57:52 +0100
From:	Heiko Carstens <heiko.carstens@...ibm.com>
To:	chenliu@...et.uwaterloo.ca
Cc:	schwidefsky@...ibm.com, linux390@...ibm.com, cotte@...ibm.com,
	linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH]early: Fix possible overlapping data buffer

On Mon, Mar 14, 2011 at 11:25:32AM -0400, chenliu@...et.uwaterloo.ca wrote:
> Thanks Heiko. This patch hasn't been tested yet. I've modifed
> the Signed=off-by to make it identical to From. Here is the
> patch:

Yes, obviously untested (see below).

> Signed-off-by: Chen Liu <chenliu@...et.uwaterloo.ca>
> ---
>  arch/s390/kernel/early.c |   16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/kernel/early.c b/arch/s390/kernel/early.c
> --- a/arch/s390/kernel/early.c
> +++ b/arch/s390/kernel/early.c
> @@ -94,6 +94,7 @@ static noinline __init void create_kerne
>  	unsigned int sinitrd_pfn, einitrd_pfn;
>  #endif
>  	int response;
> +	int hlen;
>  	size_t len;
>  	char *savesys_ptr;
>  	char defsys_cmd[DEFSYS_CMD_SIZE];
> @@ -124,22 +125,27 @@ static noinline __init void create_kerne
>  	end_pfn = PFN_UP(__pa(&_end));
>  	min_size = end_pfn << 2;
> 
> -	sprintf(defsys_cmd, "DEFSYS %s 00000-%.5X EW %.5X-%.5X SR %.5X-%.5X",
> +	snprintf(defsys_cmd, sizeof(defsys_cmd),
> +		"DEFSYS %s 00000-%.5X EW %.5X-%.5X SR %.5X-%.5X",
>  		kernel_nss_name, stext_pfn - 1, stext_pfn, eshared_pfn - 1,
>  		eshared_pfn, end_pfn);
> +	defsys_cmd[DEFSYS_CMD_SIZE - 1] = '\0';
> 
>  #ifdef CONFIG_BLK_DEV_INITRD
>  	if (INITRD_START && INITRD_SIZE) {
>  		sinitrd_pfn = PFN_DOWN(__pa(INITRD_START));
>  		einitrd_pfn = PFN_UP(__pa(INITRD_START + INITRD_SIZE));
>  		min_size = einitrd_pfn << 2;
> -		sprintf(defsys_cmd, "%s EW %.5X-%.5X", defsys_cmd,
> -		sinitrd_pfn, einitrd_pfn);
> +		hlen += snprintf(defsys_cmd, DEFSYS_CMD_SIZE - hlen,

hlen is unitialized here, since you forgot to save the size in the
snprintf statement above.

> +			" EW %.5X-%.5X", defsys_cmd,

defsys_cmd is still in the input parameter list. The whole point was to
remove it. ;)

Anyway, I fixed it and applied your patch. Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/