Skip to content

Commit

Permalink
Revert "fs/exec: allow to unshare a time namespace on vfork+exec"
Browse files Browse the repository at this point in the history
This reverts commit 133e2d3.

Alexey pointed out a few undesirable side effects of the reverted change.
First, it doesn't take into account that CLONE_VFORK can be used with
CLONE_THREAD. Second, a child process doesn't enter a target time name-space,
if its parent dies before the child calls exec. It happens because the parent
clears vfork_done.

Eric W. Biederman suggests installing a time namespace as a task gets a new mm.
It includes all new processes cloned without CLONE_VM and all tasks that call
exec(). This is an user API change, but we think there aren't users that depend
on the old behavior.

It is too late to make such changes in this release, so let's roll back
this patch and introduce the right one in the next release.

Cc: Alexey Izbyshev <[email protected]>
Cc: Christian Brauner <[email protected]>
Cc: Dmitry Safonov <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: Florian Weimer <[email protected]>
Cc: Kees Cook <[email protected]>
Signed-off-by: Andrei Vagin <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
avagin authored and kees committed Sep 13, 2022
1 parent 2b1e892 commit 33a2d6b
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 13 deletions.
7 changes: 0 additions & 7 deletions fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@
#include <linux/io_uring.h>
#include <linux/syscall_user_dispatch.h>
#include <linux/coredump.h>
#include <linux/time_namespace.h>

#include <linux/uaccess.h>
#include <asm/mmu_context.h>
Expand Down Expand Up @@ -979,12 +978,10 @@ static int exec_mmap(struct mm_struct *mm)
{
struct task_struct *tsk;
struct mm_struct *old_mm, *active_mm;
bool vfork;
int ret;

/* Notify parent that we're no longer interested in the old VM */
tsk = current;
vfork = !!tsk->vfork_done;
old_mm = current->mm;
exec_mm_release(tsk, old_mm);
if (old_mm)
Expand Down Expand Up @@ -1029,10 +1026,6 @@ static int exec_mmap(struct mm_struct *mm)
tsk->mm->vmacache_seqnum = 0;
vmacache_flush(tsk);
task_unlock(tsk);

if (vfork)
timens_on_fork(tsk->nsproxy, tsk);

if (old_mm) {
mmap_read_unlock(old_mm);
BUG_ON(active_mm != old_mm);
Expand Down
5 changes: 1 addition & 4 deletions kernel/fork.c
Original file line number Diff line number Diff line change
Expand Up @@ -2046,11 +2046,8 @@ static __latent_entropy struct task_struct *copy_process(
/*
* If the new process will be in a different time namespace
* do not allow it to share VM or a thread group with the forking task.
*
* On vfork, the child process enters the target time namespace only
* after exec.
*/
if ((clone_flags & (CLONE_VM | CLONE_VFORK)) == CLONE_VM) {
if (clone_flags & (CLONE_THREAD | CLONE_VM)) {
if (nsp->time_ns != nsp->time_ns_for_children)
return ERR_PTR(-EINVAL);
}
Expand Down
3 changes: 1 addition & 2 deletions kernel/nsproxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
if (IS_ERR(new_ns))
return PTR_ERR(new_ns);

if ((flags & CLONE_VM) == 0)
timens_on_fork(new_ns, tsk);
timens_on_fork(new_ns, tsk);

tsk->nsproxy = new_ns;
return 0;
Expand Down

0 comments on commit 33a2d6b

Please sign in to comment.