linux-kernel - Re: 2.6.18-mm2 - oops in cache_alloc

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: <20061003163415.GA17252@bougret.hpl.hp.com>
Date:	Tue, 3 Oct 2006 09:34:15 -0700
From:	Jean Tourrilhes <jt@....hp.com>
To:	Samuel Tardieu <sam@...1149.net>
Cc:	Pavel Roskin <proski@....org>,
	"John W. Linville" <linville@...driver.com>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: 2.6.18-mm2 - oops in cache_alloc_refill()

On Tue, Oct 03, 2006 at 05:58:31PM +0200, Samuel Tardieu wrote:
> >>>>> "Jean" == Jean Tourrilhes <jt@....hp.com> writes:
> 
> Jean> @@ -2500,9 +2501,9 @@ static int orinoco_hw_get_essid(struct o
> Jean>  	len = le16_to_cpu(essidbuf.len);
> Jean>  	BUG_ON(len > IW_ESSID_MAX_SIZE);
> Jean>  
> Jean> -	memset(buf, 0, IW_ESSID_MAX_SIZE+1);
> Jean> +	memset(buf, 0, IW_ESSID_MAX_SIZE);
> Jean>  	memcpy(buf, p, len);
> Jean> -	buf[len] = '\0';
> Jean> +	err = len;
> 
> Jean,
> 
> something bugs me here:
> 
>   - either buf is supposed to be a nul-terminated string, in which
>     case if p is IW_ESSID_MAX_SIZE long there may be a bug (no '\0' at
>     the end of buf)

	ESSID is supposed to be up to 32 char, so we need to full
buffer size.

>   - either buf is not-supposed to be nul-terminated and the length
>     value will always be used, in which case the memset() looks
>     useless

	Yes, it is entirely useless, but not incorrect. Note that the
code was not very efficient to start with, the last char of the string
was set to NUL twice.
	I don't really want to overstep my authority there, my goal
was to minimise the changes. Pavel will have to clean up my mess, so I
don't want change things too much.

> I suggest that you revert the memset() to IW_ESSID_MAX_SIZE+1 so that
> the last byte is cleared as well. Or am I missing something?

	No, that would bring back the slab/memory overflow we are
trying to get rid of.

>  Sam
> -- 
> Samuel Tardieu -- sam@...1149.net -- http://www.rfc1149.net/

	Strange, this name remind me someone. Must be a previous life ;-)

	A+

	Jean
-
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/