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