-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
env_dup: fix copying of parent's env to child with ARCH_ADDRENV=y #6010
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pussuw
changed the title
Env dup fix
env_dup: fix copying of parent's env to child with ARCH_ADDRENV=y
Apr 8, 2022
CI error is due to unused label: Will fix it after comments |
pussuw
force-pushed
the
env_dup_fix
branch
2 times, most recently
from
April 12, 2022 07:03
a63e1ae
to
8561eb1
Compare
pussuw
commented
Apr 12, 2022
pussuw
force-pushed
the
env_dup_fix
branch
5 times, most recently
from
April 13, 2022 08:23
4aec903
to
e9517e7
Compare
pussuw
force-pushed
the
env_dup_fix
branch
2 times, most recently
from
April 19, 2022 10:35
f44dc17
to
ee6874b
Compare
pussuw
commented
Apr 19, 2022
pussuw
commented
Apr 19, 2022
pussuw
commented
Apr 19, 2022
pussuw
force-pushed
the
env_dup_fix
branch
3 times, most recently
from
April 19, 2022 12:19
419ca36
to
e75e9fb
Compare
Ensures that when errout_xx is taken, args are freed
pussuw
force-pushed
the
env_dup_fix
branch
4 times, most recently
from
April 20, 2022 08:40
a38b664
to
a832d1b
Compare
xiaoxiang781216
approved these changes
Apr 21, 2022
If address environments are in use, it is not possible to simply memcpy from from one process to another. The current implementation of env_dup does precisely this and thus, it fails at once when it is attempted between two user processes. The solution is to use the kernel's heap as an intermediate buffer. This is a simple, effective and common way to do a fork(). Obviously this is not needed for kernel processes.
Now that the environment strings are stored as an array of strings, they can be passed from the application via char **environ, which is really defined as a function call to get_environ_ptr(). This works as is for flat build, but protected mode and kernel mode require a call gate, which is added here.
This keeps backwards compatibility with apps that do not provide envp. The old implementation passes NULL always and this change fixes any regression caused by it.
I patched two build errors due to missing definition of get_environ_ptr when CONFIG_DISABLE_ENVIRON is defined. |
xiaoxiang781216
approved these changes
Apr 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes copying of parent environment to child when address environments are in use. This is done by copying the parent's env into neutral kernel memory first, and from kernel memory to the child.
Impact
Fixes environment variables for ARCH_ADDRENV=y and BUILD_KERNEL=y
Testing
MPFS icicle:knsh