Skip to content

Commit 2b5f9da

Browse files
avaginkees
authored andcommitted
fs/exec: switch timens when a task gets a new mm
Changing a time namespace requires remapping a vvar page, so we don't want to allow doing that if any other tasks can use the same mm. Currently, we install a time namespace when a task is created with a new vm. exec() is another case when a task gets a new mm and so it can switch a time namespace safely, but it isn't handled now. One more issue of the current interface is that clone() with CLONE_VM isn't allowed if the current task has unshared a time namespace (timens_for_children doesn't match the current timens). Both these issues make some inconvenience for users. For example, Alexey and Florian reported that posix_spawn() uses vfork+exec and this pattern doesn't work with time namespaces due to the both described issues. LXC needed to workaround the exec() issue by calling setns. In the commit 133e2d3 ("fs/exec: allow to unshare a time namespace on vfork+exec"), we tried to fix these issues with minimal impact on UAPI. But it adds extra complexity and some undesirable side effects. Eric suggested fixing the issues properly because here are all the reasons to suppose that there are no users that depend on the old behavior. 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]> Suggested-by: "Eric W. Biederman" <[email protected]> Origin-author: "Eric W. Biederman" <[email protected]> Signed-off-by: Andrei Vagin <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 594d2a1 commit 2b5f9da

File tree

4 files changed

+27
-11
lines changed

4 files changed

+27
-11
lines changed

fs/exec.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
#include <linux/io_uring.h>
6565
#include <linux/syscall_user_dispatch.h>
6666
#include <linux/coredump.h>
67+
#include <linux/time_namespace.h>
6768

6869
#include <linux/uaccess.h>
6970
#include <asm/mmu_context.h>
@@ -1297,6 +1298,10 @@ int begin_new_exec(struct linux_binprm * bprm)
12971298

12981299
bprm->mm = NULL;
12991300

1301+
retval = exec_task_namespaces();
1302+
if (retval)
1303+
goto out_unlock;
1304+
13001305
#ifdef CONFIG_POSIX_TIMERS
13011306
spin_lock_irq(&me->sighand->siglock);
13021307
posix_cpu_timers_exit(me);

include/linux/nsproxy.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ static inline struct cred *nsset_cred(struct nsset *set)
9494
int copy_namespaces(unsigned long flags, struct task_struct *tsk);
9595
void exit_task_namespaces(struct task_struct *tsk);
9696
void switch_task_namespaces(struct task_struct *tsk, struct nsproxy *new);
97+
int exec_task_namespaces(void);
9798
void free_nsproxy(struct nsproxy *ns);
9899
int unshare_nsproxy_namespaces(unsigned long, struct nsproxy **,
99100
struct cred *, struct fs_struct *);

kernel/fork.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,15 +2043,6 @@ static __latent_entropy struct task_struct *copy_process(
20432043
return ERR_PTR(-EINVAL);
20442044
}
20452045

2046-
/*
2047-
* If the new process will be in a different time namespace
2048-
* do not allow it to share VM or a thread group with the forking task.
2049-
*/
2050-
if (clone_flags & (CLONE_THREAD | CLONE_VM)) {
2051-
if (nsp->time_ns != nsp->time_ns_for_children)
2052-
return ERR_PTR(-EINVAL);
2053-
}
2054-
20552046
if (clone_flags & CLONE_PIDFD) {
20562047
/*
20572048
* - CLONE_DETACHED is blocked so that we can potentially

kernel/nsproxy.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
157157
if (likely(!(flags & (CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |
158158
CLONE_NEWPID | CLONE_NEWNET |
159159
CLONE_NEWCGROUP | CLONE_NEWTIME)))) {
160-
if (likely(old_ns->time_ns_for_children == old_ns->time_ns)) {
160+
if ((flags & CLONE_VM) ||
161+
likely(old_ns->time_ns_for_children == old_ns->time_ns)) {
161162
get_nsproxy(old_ns);
162163
return 0;
163164
}
@@ -179,7 +180,8 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
179180
if (IS_ERR(new_ns))
180181
return PTR_ERR(new_ns);
181182

182-
timens_on_fork(new_ns, tsk);
183+
if ((flags & CLONE_VM) == 0)
184+
timens_on_fork(new_ns, tsk);
183185

184186
tsk->nsproxy = new_ns;
185187
return 0;
@@ -254,6 +256,23 @@ void exit_task_namespaces(struct task_struct *p)
254256
switch_task_namespaces(p, NULL);
255257
}
256258

259+
int exec_task_namespaces(void)
260+
{
261+
struct task_struct *tsk = current;
262+
struct nsproxy *new;
263+
264+
if (tsk->nsproxy->time_ns_for_children == tsk->nsproxy->time_ns)
265+
return 0;
266+
267+
new = create_new_namespaces(0, tsk, current_user_ns(), tsk->fs);
268+
if (IS_ERR(new))
269+
return PTR_ERR(new);
270+
271+
timens_on_fork(new, tsk);
272+
switch_task_namespaces(tsk, new);
273+
return 0;
274+
}
275+
257276
static int check_setns_flags(unsigned long flags)
258277
{
259278
if (!flags || (flags & ~(CLONE_NEWNS | CLONE_NEWUTS | CLONE_NEWIPC |

0 commit comments

Comments
 (0)