libc: add cleanenv function

[oshogbo]

Summary

The clearenv(3) function allows us to clear all environment variable in one
shot. This may be useful for security programs that want to control
the environment or what variables are passed to new spawned programs.

[kib]

Look at src/MAINTAINERS file for the *env(3) line.

lib/libc/stdlib/Symbol.map

129

This should be alphabetically sorted.

lib/libc/stdlib/getenv.3

228

I doubt it ('first appeared in glibc'). Linux man page states

CONFORMING TO
       Various UNIX variants (DG/UX, HP-UX, QNX, ...).  POSIX.9 (bindings  for
       FORTRAN77).

[oshogbo]

Thank you @kib for hints!

[scf]

Looks pretty good. I have some ideas/suggestions.

BTW, good job at stomaching that code! ;)

lib/libc/stdlib/getenv.c

696

Suggestion: Unset all variables by flagging them ...

704–715

It has been many years since I have looked at this code, so bear with me. :)

There may be two other options, which you may have already looked at:

• Calling environ = NULL and calling __merge_environ() may accomplish the same.
• Calling __clean_env(false) and setting environ = NULL.

Those may work, but you would have to test to make sure.

715

The final call of __rebuild_environ(0) at the end of the function will have environ set to intEnviron which is not NULL at that point. The Linux man page says environ will be set to NULL. I also recommend a unit test for that.

[0mp]

I am not sure but shouldn't we be adding "SPDX-License-Identifier: BSD-2-Clause-FreeBSD" when committing new files?

lib/libc/stdlib/getenv.3

39

Shouldn't the Makefile be edited as well to install a symlink for clearenv.3?

[oshogbo]

Update diff.

[oshogbo]

@0mp Thanks I addressed that.

lib/libc/stdlib/getenv.c

715

Yea that looks much easier. I did like you suggested and added some more tests.

[oshogbo]

Ping?

[scf]

Sorry for the long turnaround time on the review. Thank you for adding more tests.

include/stdlib.h

275

I think this declaration should be placed just above the daemon() declaration later in the file to preserve the sorted order. Just a blank line above it and no blank line between it and daemon(). It would be nice for someone more knowledgeable to agree or disagree with this. It just looks like it matches the flow better.

lib/libc/stdlib/getenv.3

70

I believe you meant putenv instead of getenv.

lib/libc/tests/stdlib/clearenv_test.c

132

I am just verifying. Is this last line testing recreation after clearing twice, or is it due to copying another test such as clearenv__recreated_vars_test()?

145

This is missing a blank line.

154

I suggest a test or more around putenv() too. At least, tests that verify that variables put into environ are cleared and the original string passed via putenv() is untouched.

[oshogbo]

Address scf@ suggestions.

[oshogbo]

lib/libc/tests/stdlib/clearenv_test.c

132

Correct, I'm verifying that after duble clear we still can create new values.

[oshogbo]

Ping?

[oshogbo]

@scf secteam Can I proceed with this?

[markj]

lib/libc/stdlib/getenv.c

710

Don't you need to adjust envActive here?

[oshogbo]

lib/libc/stdlib/getenv.c

710

As far as I understand the __rebuild_environ is adjusting it. In unsetenv we also don't adjust envActive.

[emaste]

include/stdlib.h

275

@scf's comment makes sense to me

lib/libc/stdlib/getenv.c

696

agreed

[oshogbo]

Address @scf and @emaste suggestions.

[oshogbo]

@secteam ping?

[delphij]

lib/libc/stdlib/Symbol.map

125

Minor nit: This should be added to FBSD_1.7 namespace (I really wish we can have the future new namespaces named after the major release..)