Date: Sun, 17 Jul 2005 21:14:49 -0500 From: Harry Coin <harrycoin@qconline.com> To: freebsd-current@freebsd.org Subject: kern_environment.c routine KENV_SET setenv writes past kenvp, kernel mem without limit? Message-ID: <4.3.2.7.2.20050717205531.01f2bf30@mail.qconline.com> In-Reply-To: <20050717120016.99E4716A41C@hub.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4.3.2.7.2.20050717205531.01f2bf30>