From owner-svn-src-stable-11@freebsd.org Fri Dec 13 03:29:55 2019 Return-Path: Delivered-To: svn-src-stable-11@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 771AF1E1192; Fri, 13 Dec 2019 03:29:55 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47Yx4M2J3hz4RFK; Fri, 13 Dec 2019 03:29:55 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4A0E760F4; Fri, 13 Dec 2019 03:29:55 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id xBD3Ttgh088976; Fri, 13 Dec 2019 03:29:55 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id xBD3TtOk088975; Fri, 13 Dec 2019 03:29:55 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201912130329.xBD3TtOk088975@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Fri, 13 Dec 2019 03:29:55 +0000 (UTC) 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 X-SVN-Group: stable-11 X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: in stable: 11/sys/kern 12/sys/kern X-SVN-Commit-Revision: 355689 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Dec 2019 03:29:55 -0000 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