On Tue, 5 Feb 2019, Kyle Evans wrote:

On Tue, Feb 5, 2019 at 9:35 AM Bruce Evans <b...@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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to