Date: Fri, 13 Dec 2019 03:29:55 +0000 (UTC) From: Kyle Evans <kevans@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r355689 - in stable: 11/sys/kern 12/sys/kern Message-ID: <201912130329.xBD3TtOk088975@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: kevans Date: Fri Dec 13 03:29:54 2019 New Revision: 355689 URL: https://svnweb.freebsd.org/changeset/base/355689 Log: MFC r343777, r352244-r352245: kenv fix + assertions r343777: Fix zapping of static hints and env in init_static_kenv(). Environments are terminated by 2 NULs, but only 1 NUL was zapped. Zapping only 1 NUL just splits the first string into an empty string and a corrupted string. All other strings in static hints and env remained live early in the boot when they were supposed to be disabled. Support calling init_static_kenv() very early in the boot, so as to use the env very early in the boot. Then the pointer to the loader env may change after the first call due to enabling paging or otherwise remapping the pointer. Another call is needed to register the change. Don't use the previous pointer in this (or any) later call. r352244: kenv: assert that an empty static buffer passed in is "empty" Garbage in the passed-in buffer can cause problems if any attempts to read the kenv are inadvertently made between init_static_kenv and the first kern_setenv -- assuming there is one. This is cheap and easy, so do it. This also helps rule out some class of bugs as one tries to debug; tunables fetch from the static environment up until SI_SUB_KMEM + 1, and many of these buffers are global ~4k buffers that rely on BSS clearing while others just grab a page of free memory and use it (e.g. xen). r352245: Follow up r352244: kenv: tighten up assertions As I like to forget: static kenv var formatting is actually such that an empty environment would be double null bytes. We should make sure that a non-zero buffer has at least enough for this, though most of the current usage is with a 4k buffer. Modified: stable/11/sys/kern/kern_environment.c Directory Properties: stable/11/ (props changed) Changes in other areas also in this revision: Modified: stable/12/sys/kern/kern_environment.c Directory Properties: stable/12/ (props changed) Modified: stable/11/sys/kern/kern_environment.c ============================================================================== --- stable/11/sys/kern/kern_environment.c Fri Dec 13 02:20:26 2019 (r355688) +++ stable/11/sys/kern/kern_environment.c Fri Dec 13 03:29:54 2019 (r355689) @@ -249,6 +249,33 @@ init_static_kenv(char *buf, size_t len) KASSERT(!dynamic_kenv, ("kenv: dynamic_kenv already initialized")); /* + * Suitably sized means it must be able to hold at least one empty + * variable, otherwise things go belly up if a kern_getenv call is + * made without a prior call to kern_setenv as we have a malformed + * environment. + */ + KASSERT(len == 0 || len >= 2, + ("kenv: static env must be initialized or suitably sized")); + KASSERT(len == 0 || (*buf == '\0' && *(buf + 1) == '\0'), + ("kenv: sized buffer must be initially empty")); + + /* + * We may be called twice, with the second call needed to relocate + * md_envp after enabling paging. md_envp is then garbage if it is + * not null and the relocation will move it. Discard it so as to + * not crash using its old value in our first call to kern_getenv(). + * + * The second call gives the same environment as the first except + * in silly configurations where the static env disables itself. + * + * Other env calls don't handle possibly-garbage pointers, so must + * not be made between enabling paging and calling here. + */ + md_envp = NULL; + md_env_len = 0; + md_env_pos = 0; + + /* * Give the static environment a chance to disable the loader(8) * environment first. This is done with loader_env.disabled=1. * @@ -284,12 +311,16 @@ init_static_kenv(char *buf, size_t len) md_env_pos = 0; eval = kern_getenv("static_env.disabled"); - if (eval != NULL && strcmp(eval, "1") == 0) - *static_env = '\0'; + if (eval != NULL && strcmp(eval, "1") == 0) { + static_env[0] = '\0'; + static_env[1] = '\0'; + } eval = kern_getenv("static_hints.disabled"); - if (eval != NULL && strcmp(eval, "1") == 0) - *static_hints = '\0'; + if (eval != NULL && strcmp(eval, "1") == 0) { + static_hints[0] = '\0'; + static_hints[1] = '\0'; + } /* * Now we see if we need to tear the loader environment back down due
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201912130329.xBD3TtOk088975>