Skip to content
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
merged 4 commits into from
Apr 21, 2022

Conversation

pussuw
Copy link
Contributor

@pussuw pussuw commented Apr 8, 2022

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

@pussuw pussuw changed the title Env dup fix env_dup: fix copying of parent's env to child with ARCH_ADDRENV=y Apr 8, 2022
@pussuw
Copy link
Contributor Author

pussuw commented Apr 8, 2022

CI error is due to unused label:
Error: binfmt_execmodule.c:301:1: error: label 'errout_with_args' defined but not used [-Werror=unused-label]

Will fix it after comments

binfmt/binfmt_copyenv.c Outdated Show resolved Hide resolved
binfmt/binfmt_execmodule.c Outdated Show resolved Hide resolved
binfmt/binfmt_execmodule.c Show resolved Hide resolved
binfmt/binfmt_execmodule.c Show resolved Hide resolved
binfmt/binfmt_execmodule.c Outdated Show resolved Hide resolved
binfmt/binfmt_execmodule.c Outdated Show resolved Hide resolved
sched/task/task_init.c Outdated Show resolved Hide resolved
sched/environ/env_dup.c Show resolved Hide resolved
@pussuw pussuw force-pushed the env_dup_fix branch 2 times, most recently from a63e1ae to 8561eb1 Compare April 12, 2022 07:03
@pussuw pussuw marked this pull request as draft April 12, 2022 08:26
@pussuw pussuw marked this pull request as ready for review April 12, 2022 09:14
sched/environ/env_dup.c Outdated Show resolved Hide resolved
@pussuw pussuw force-pushed the env_dup_fix branch 5 times, most recently from 4aec903 to e9517e7 Compare April 13, 2022 08:23
sched/group/group.h Outdated Show resolved Hide resolved
sched/group/group_env.c Outdated Show resolved Hide resolved
sched/group/group_env.c Outdated Show resolved Hide resolved
sched/group/group_env.c Outdated Show resolved Hide resolved
sched/task/task_vfork.c Outdated Show resolved Hide resolved
sched/task/task_vfork.c Outdated Show resolved Hide resolved
binfmt/binfmt_exec.c Outdated Show resolved Hide resolved
@xiaoxiang781216
Copy link
Contributor

@pussuw the new env implementation is here: #6083. Please review it.

binfmt/binfmt.h Outdated Show resolved Hide resolved
@pussuw pussuw force-pushed the env_dup_fix branch 2 times, most recently from f44dc17 to ee6874b Compare April 19, 2022 10:35
sched/task/task_posixspawn.c Outdated Show resolved Hide resolved
sched/task/task_vfork.c Outdated Show resolved Hide resolved
@pussuw pussuw force-pushed the env_dup_fix branch 3 times, most recently from 419ca36 to e75e9fb Compare April 19, 2022 12:19
binfmt/binfmt_exec.c Show resolved Hide resolved
sched/task/task_posixspawn.c Outdated Show resolved Hide resolved
sched/task/task_create.c Outdated Show resolved Hide resolved
sched/environ/environ.h Outdated Show resolved Hide resolved
sched/environ/environ.h Outdated Show resolved Hide resolved
Ensures that when errout_xx is taken, args are freed
@pussuw pussuw force-pushed the env_dup_fix branch 4 times, most recently from a38b664 to a832d1b Compare April 20, 2022 08:40
binfmt/binfmt_execmodule.c Show resolved Hide resolved
sched/environ/env_dup.c Outdated Show resolved Hide resolved
pussuw added 3 commits April 21, 2022 11:53
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.
@pussuw
Copy link
Contributor Author

pussuw commented Apr 21, 2022

I patched two build errors due to missing definition of get_environ_ptr when CONFIG_DISABLE_ENVIRON is defined.

@xiaoxiang781216 xiaoxiang781216 merged commit 1a7d81c into apache:master Apr 21, 2022
@pussuw pussuw deleted the env_dup_fix branch April 21, 2022 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants