From owner-svn-src-head@freebsd.org Tue Feb 5 17:48:51 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D326314C2460; Tue, 5 Feb 2019 17:48:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 4BC6385AAD; Tue, 5 Feb 2019 17:48:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id 0C2D43D8E9A; Wed, 6 Feb 2019 04:48:47 +1100 (AEDT) Date: Wed, 6 Feb 2019 04:48:46 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Kyle Evans cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r343777 - head/sys/kern In-Reply-To: Message-ID: <20190206042358.E3786@besplex.bde.org> References: <201902051534.x15FYtZU066605@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=dQAJO8qa6FECyPOhYHEA:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-Rspamd-Queue-Id: 4BC6385AAD X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.96 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.96)[-0.960,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Feb 2019 17:48:51 -0000 On Tue, 5 Feb 2019, Kyle Evans wrote: > On Tue, Feb 5, 2019 at 9:35 AM Bruce Evans wrote: >> ... >> Log: >> 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. > > I think we need to go another step here. This stuff was functional in > my testing because it was all late enough to happen after static_env > and static_hints were merged into the dynamic kenv (which I've only > now noticed after you fixed this). It looks like our logic for merging > is broken, IMO. It was too early to work in hammer_time() and init386() where important tunables are looked up. E.g., dynamic kenv needs malloc, but in these functions even the memory size isn't quite known and it is controlled by the hw.physmem tunable. I missed this since I don't use the merging feature and usually duplicate the static hints in the dynamic hints. > Before I touched it: > > - When static_hints did get merged (by toggling of sysctl) it would > stop merging at the first empty string (strlen(cp) == 0) -- introduced > in r240067 -- regardless of whether said empty string was followed by > a second NUL terminator. I think the syntax of the config file doesn't allow creating empty strings in the middle, so this worked. > - When static_env merged in at SU_SUB_KMEM, it wouldn't merge if > *kern_envp == '\0' but it wouldn't stop at an empty string, instead > carrying the empty string into the dynamic env if my reading is > correct. > > I broke the former even further by not merging anything at all if > *static_hints == '\0', and I maintained the latter breakage except > added an additional warning if we ventured upon a malformed entry. I thought that the dynamic env initialization dropped the misformatted static hints and env more intentionally. > Both of these are inconsistent with how the environments are observed > by kern_getenv or hints consumers before the merging, which will > simply skip over the malformed empty strings until it hits proper > termination. I think the resulting environment should be consistent > with what these consumers would've seen pre-merge, and I think this > should be fixed, if we can. I think we can trust the compile-time hints and envs to not have empty strings (or even ones not in the form name=value). Then don't mess them up by zapping them but instead start with a compile-time initialization of a pointer to them and zap that. The pointer can be null and the hints and env don't even need to exist when they are empty. _getenv_static() already works right with null pointers. Hints looks like it needs more reorganization. Bruce