Date: Thu, 23 Oct 2014 13:01:18 -0700 From: Colin Percival <cperciva@freebsd.org> To: =?UTF-8?B?SmVhbi1Tw6liYXN0aWVuIFDDqWRyb24=?= <dumbbell@FreeBSD.org>, svn-src-all@freebsd.org Subject: Re: svn commit: r273487 - head/sys/kern Message-ID: <54495E8E.20408@freebsd.org> In-Reply-To: <5448F973.8050102@FreeBSD.org> References: <201410222335.s9MNZW62045167@svn.freebsd.org> <5448F973.8050102@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/23/14 05:49, Jean-S=C3=A9bastien P=C3=A9dron wrote:=0D
> The following change triggers a kernel trap 12 when env is NULL:=0D
> =0D
>> @@ -260,8 +262,10 @@ void=0D
>> freeenv(char *env)=0D
>> {=0D
>> =0D
>> - if (dynamic_kenv)=0D
>> + if (dynamic_kenv) {=0D
>> + memset(env, 0, strlen(env));=0D
>> free(env, M_KENV);=0D
>> + }=0D
>> }=0D
> =0D
> This happens very early in boot for me, just after the lines:=0D
> WARNING: WITNESS option enabled, expect reduced performance.=0D
> VT: running with driver "vga".=0D
=0D
This sounds like a bug in the code which is using kern_getenv / freeenv.=0D
The comment at kern_getenv says=0D
* Look up an environment variable by name.=0D
* Return a pointer to the string if found.=0D
* The pointer has to be freed with freeenv()=0D
* after use.=0D
which I interpret to mean that if the environment variable is not found=0D
and you don't get a pointer to a string, you shouldn't be freeing it.=0D
=0D
I'm willing to work around this in freeenv, but since we're in HEAD and=0D
this isn't going to be MFCed, it seems like an opportunity to fix the=0D
code which is calling freeenv(NULL).=0D
=0D
> The attached simple patch fixes the problem.=0D
> =0D
> What I don't know is if the same problem can occur in kern_unsetenv():=0D
> =0D
>> @@ -437,6 +441,7 @@ kern_unsetenv(const char *name)=0D
>> kenvp[i++] =3D kenvp[j];=0D
>> kenvp[i] =3D NULL;=0D
>> mtx_unlock(&kenv_lock);=0D
>> + memset(oldenv, 0, strlen(oldenv));=0D
>> free(oldenv, M_KENV);=0D
>> return (0);=0D
>> }=0D
=0D
We can only get into that code if cp !=3D NULL, and cp is part of the=0D
string pointed to by kenvp[i] (aka. oldenv) so unless I'm missing=0D
something we're guaranteed that oldenv is a pointer to a string here.=0D
=0D
Colin Percival
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54495E8E.20408>