svn commit: r273487 - head/sys/kern

[Colin Percival]

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