kern_environment.c routine KENV_SET setenv writes past kenvp, kernel mem without limit?

From: Harry Coin <harrycoin_at_qconline.com>
Date: Sun, 17 Jul 2005 21:14:49 -0500
See kern_environment.c, note that the various routines that add to the 
kernel environment wind up calling setenv.

There is a define at the top showing the kernel environment array has a 
fixed 512 entries.

That define is used once, when the array is initialized.  There is, 
apparently, no overflow check.

Calling setenv, which it appears one can do from the user space, appears to 
plow right past the end of the array in kernel space.

I don't think it is a security hole, as the values written are always from 
a malloc, but I can it crash the system?

Am I missing something?  Suggested patch below.

P.S. I noticed this as I think the authors of sound card drivers have a 
much better idea of what good default mixer volume / gain values are (0db 
gain) and also mono speaker audio routing, etc. than the rather lame 
defaults in mixer.c.   Better sound chips and even older sound chips fully 
implemented just so exceed the roster of pre-canned mixer device names. The 
idea of using device hints seems specific only to 'pcm' and not to the 
driver. One size does not fit all.   So I'm trying to have the driver's 
mixer init routine add device hints for default volumes but only if none 
are present prior.   It does not go well.

Harry

(whole routine at 
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/kern_environment.c?rev=1.39&content-type=text/x-cvsweb-markup&only_with_tag=MAIN 
)

int
setenv(const char *name, const char *value)
{
         char *buf, *cp, *oldenv;
         int namelen, vallen, i;

         KENV_CHECK;

         namelen = strlen(name) + 1;
         if (namelen > KENV_MNAMELEN)
                 return (-1);
         vallen = strlen(value) + 1;
         if (vallen > KENV_MVALLEN)
                 return (-1);
         buf = malloc(namelen + vallen, M_KENV, M_WAITOK);
         sprintf(buf, "%s=%s", name, value);

         sx_xlock(&kenv_lock);
         cp = _getenv_dynamic(name, &i);
         if (cp != NULL) {
                 oldenv = kenvp[i];
                 kenvp[i] = buf;
                 sx_xunlock(&kenv_lock);
                 free(oldenv, M_KENV);
         } else {
                 /* We add the option if it wasn't found */
                 for (i = 0; (cp = kenvp[i]) != NULL; i++)
                         ;
suggest :  if (i>=KENV_SIZE-1) 
{        sx_xunlock(&kenv_lock);  free(buf,M_KENV); return -1; }

                 kenvp[i] = buf;
                 kenvp[i + 1] = NULL;
                 sx_xunlock(&kenv_lock);
         }
         return (0);
}
Received on Mon Jul 18 2005 - 00:14:56 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:39 UTC