svn commit: r343777 - head/sys/kern

[Bruce Evans]

On Tue, 5 Feb 2019, Kyle Evans wrote:

> On Tue, Feb 5, 2019 at 9:35 AM Bruce Evans <bde at freebsd.org> 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