Date: Fri, 24 Oct 2014 01:21:33 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: d@delphij.net Cc: svn-src-head@freebsd.org, Dag-Erling =?utf-8?B?U23DuHJncmF2?= <des@des.no>, svn-src-all@freebsd.org, src-committers@freebsd.org, Xin LI <delphij@FreeBSD.org> Subject: Re: svn commit: r273552 - head/sys/kern Message-ID: <20141023232133.GA11222@dft-labs.eu> In-Reply-To: <54498CE9.1090000@delphij.net> References: <201410231823.s9NINpXD082633@svn.freebsd.org> <86egtyz9mz.fsf@nine.des.no> <54498CE9.1090000@delphij.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 23, 2014 at 04:19:05PM -0700, Xin Li wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> On 10/23/14 15:47, Dag-Erling Smørgrav wrote:
> > Xin LI <delphij@FreeBSD.org> writes:
> >> Log: Test if 'env' is NULL before doing memset() and strlen(),
> >> the caller may pass NULL to freeenv().
> >
> > If this is in response to a panic in early boot, the real bug is
> > elsewhere (see r273564). Adding a NULL check here only hides it.
>
> Yes that would fix it. Does this look good to you?
>
> Index: sys/kern/kern_environment.c
> ===================================================================
> - --- sys/kern/kern_environment.c (revision 273564)
> +++ sys/kern/kern_environment.c (working copy)
> @@ -262,7 +262,8 @@ void
> freeenv(char *env)
> {
>
> - - if (dynamic_kenv && env != NULL) {
> + MPASS(env != NULL);
> + if (dynamic_kenv) {
> memset(env, 0, strlen(env));
> free(env, M_KENV);
> }
>
There are at least 80 consumers of this function. Unless someone is up
to reviewing them all, can we go with a warning + backtrace for the time
being?
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141023232133.GA11222>