Skip to content

Commit 3af8588

Browse files
author
Christian Brauner
committed
fork: fold legacy_clone_args_valid() into _do_fork()
This separate helper only existed to guarantee the mutual exclusivity of CLONE_PIDFD and CLONE_PARENT_SETTID for legacy clone since CLONE_PIDFD abuses the parent_tid field to return the pidfd. But we can actually handle this uniformely thus removing the helper. For legacy clone we can detect that CLONE_PIDFD is specified in conjunction with CLONE_PARENT_SETTID because they will share the same memory which is invalid and for clone3() setting the separate pidfd and parent_tid fields to the same memory is bogus as well. So fold that helper directly into _do_fork() by detecting this case. Cc: Thomas Gleixner <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Al Viro <[email protected]> Cc: Geert Uytterhoeven <[email protected]> Cc: "Matthew Wilcox (Oracle)" <[email protected]> Cc: "Peter Zijlstra (Intel)" <[email protected]> Cc: [email protected] Cc: [email protected] Signed-off-by: Christian Brauner <[email protected]>
1 parent 4877846 commit 3af8588

File tree

4 files changed

+14
-23
lines changed

4 files changed

+14
-23
lines changed

arch/m68k/kernel/process.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,6 @@ asmlinkage int m68k_clone(struct pt_regs *regs)
125125
.tls = regs->d5,
126126
};
127127

128-
if (!legacy_clone_args_valid(&args))
129-
return -EINVAL;
130-
131128
return _do_fork(&args);
132129
}
133130

arch/x86/kernel/sys_ia32.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,6 @@ COMPAT_SYSCALL_DEFINE5(ia32_clone, unsigned long, clone_flags,
251251
.tls = tls_val,
252252
};
253253

254-
if (!legacy_clone_args_valid(&args))
255-
return -EINVAL;
256-
257254
return _do_fork(&args);
258255
}
259256
#endif /* CONFIG_IA32_EMULATION */

include/linux/sched/task.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ extern void exit_files(struct task_struct *);
9696
extern void exit_itimers(struct signal_struct *);
9797

9898
extern long _do_fork(struct kernel_clone_args *kargs);
99-
extern bool legacy_clone_args_valid(const struct kernel_clone_args *kargs);
10099
extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
101100
struct task_struct *fork_idle(int);
102101
struct mm_struct *copy_init_mm(void);

kernel/fork.c

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,6 +2422,20 @@ long _do_fork(struct kernel_clone_args *args)
24222422
int trace = 0;
24232423
long nr;
24242424

2425+
/*
2426+
* For legacy clone() calls, CLONE_PIDFD uses the parent_tid argument
2427+
* to return the pidfd. Hence, CLONE_PIDFD and CLONE_PARENT_SETTID are
2428+
* mutually exclusive. With clone3() CLONE_PIDFD has grown a separate
2429+
* field in struct clone_args and it still doesn't make sense to have
2430+
* them both point at the same memory location. Performing this check
2431+
* here has the advantage that we don't need to have a separate helper
2432+
* to check for legacy clone().
2433+
*/
2434+
if ((args->flags & CLONE_PIDFD) &&
2435+
(args->flags & CLONE_PARENT_SETTID) &&
2436+
(args->pidfd == args->parent_tid))
2437+
return -EINVAL;
2438+
24252439
/*
24262440
* Determine whether and which event to report to ptracer. When
24272441
* called from kernel_thread or CLONE_UNTRACED is explicitly
@@ -2479,16 +2493,6 @@ long _do_fork(struct kernel_clone_args *args)
24792493
return nr;
24802494
}
24812495

2482-
bool legacy_clone_args_valid(const struct kernel_clone_args *kargs)
2483-
{
2484-
/* clone(CLONE_PIDFD) uses parent_tidptr to return a pidfd */
2485-
if ((kargs->flags & CLONE_PIDFD) &&
2486-
(kargs->flags & CLONE_PARENT_SETTID))
2487-
return false;
2488-
2489-
return true;
2490-
}
2491-
24922496
#ifndef CONFIG_HAVE_COPY_THREAD_TLS
24932497
/* For compatibility with architectures that call do_fork directly rather than
24942498
* using the syscall entry points below. */
@@ -2508,9 +2512,6 @@ long do_fork(unsigned long clone_flags,
25082512
.stack_size = stack_size,
25092513
};
25102514

2511-
if (!legacy_clone_args_valid(&args))
2512-
return -EINVAL;
2513-
25142515
return _do_fork(&args);
25152516
}
25162517
#endif
@@ -2593,9 +2594,6 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
25932594
.tls = tls,
25942595
};
25952596

2596-
if (!legacy_clone_args_valid(&args))
2597-
return -EINVAL;
2598-
25992597
return _do_fork(&args);
26002598
}
26012599
#endif

0 commit comments

Comments
 (0)