Skip site navigation (1)Skip section navigation (2)
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>

index | next in thread | previous in thread | raw e-mail

On 10/23/14 05:49, Jean-Sébastien Pédron wrote:

> The following change triggers a kernel trap 12 when env is NULL:

> 

>> @@ -260,8 +262,10 @@ void

>>  freeenv(char *env)

>>  {

>>  

>> -	if (dynamic_kenv)

>> +	if (dynamic_kenv) {

>> +		memset(env, 0, strlen(env));

>>  		free(env, M_KENV);

>> +	}

>>  }

> 

> This happens very early in boot for me, just after the lines:

>     WARNING: WITNESS option enabled, expect reduced performance.

>     VT: running with driver "vga".



This sounds like a bug in the code which is using kern_getenv / freeenv.

The comment at kern_getenv says

 * Look up an environment variable by name.

 * Return a pointer to the string if found.

 * The pointer has to be freed with freeenv()

 * after use.

which I interpret to mean that if the environment variable is not found

and you don't get a pointer to a string, you shouldn't be freeing it.



I'm willing to work around this in freeenv, but since we're in HEAD and

this isn't going to be MFCed, it seems like an opportunity to fix the

code which is calling freeenv(NULL).



> The attached simple patch fixes the problem.

> 

> What I don't know is if the same problem can occur in kern_unsetenv():

> 

>> @@ -437,6 +441,7 @@ kern_unsetenv(const char *name)

>>  			kenvp[i++] = kenvp[j];

>>  		kenvp[i] = NULL;

>>  		mtx_unlock(&kenv_lock);

>> +		memset(oldenv, 0, strlen(oldenv));

>>  		free(oldenv, M_KENV);

>>  		return (0);

>>  	}



We can only get into that code if cp != NULL, and cp is part of the

string pointed to by kenvp[i] (aka. oldenv) so unless I'm missing

something we're guaranteed that oldenv is a pointer to a string here.



Colin Percival


help

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?54495E8E.20408>