svn commit: r343777 - head/sys/kern

[Kyle Evans]

On Tue, Feb 5, 2019 at 9:35 AM Bruce Evans <bde at freebsd.org> wrote:
>
> Author: bde
> Date: Tue Feb  5 15:34:55 2019
> New Revision: 343777
> URL: https://svnweb.freebsd.org/changeset/base/343777
>
> 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.

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.

- 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.

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.

Thoughts?

Thanks,

Kyle Evans