Pull fork cleanups from Christian Brauner: "This is cleanup series from when we reworked a chunk of the process creation paths in the kernel and switched to struct {kernel_}clone_args. High-level this does two main things: - Remove the double export of both do_fork() and _do_fork() where do_fork() used the incosistent legacy clone calling convention. Now we only export _do_fork() which is based on struct kernel_clone_args. - Remove the copy_thread_tls()/copy_thread() split making the architecture specific HAVE_COYP_THREAD_TLS config option obsolete. This switches all remaining architectures to select HAVE_COPY_THREAD_TLS and thus to the copy_thread_tls() calling convention. The current split makes the process creation codepaths more convoluted than they need to be. Each architecture has their own copy_thread() function unless it selects HAVE_COPY_THREAD_TLS then it has a copy_thread_tls() function. The split is not needed anymore nowadays, all architectures support CLONE_SETTLS but quite a few of them never bothered to select HAVE_COPY_THREAD_TLS and instead simply continued to use copy_thread() and use the old calling convention. Removing this split cleans up the process creation codepaths and paves the way for implementing clone3() on such architectures since it requires the copy_thread_tls() calling convention. After having made each architectures support copy_thread_tls() this series simply renames that function back to copy_thread(). It also switches all architectures that call do_fork() directly over to _do_fork() and the struct kernel_clone_args calling convention. This is a corollary of switching the architectures that did not yet support it over to copy_thread_tls() since do_fork() is conditional on not supporting copy_thread_tls() (Mostly because it lacks a separate argument for tls which is trivial to fix but there's no need for this function to exist.). The do_fork() removal is in itself already useful as it allows to to remove the export of both do_fork() and _do_fork() we currently have in favor of only _do_fork(). This has already been discussed back when we added clone3(). The legacy clone() calling convention is - as is probably well-known - somewhat odd: # # ABI hall of shame # config CLONE_BACKWARDS config CLONE_BACKWARDS2 config CLONE_BACKWARDS3 that is aggravated by the fact that some architectures such as sparc follow the CLONE_BACKWARDSx calling convention but don't really select the corresponding config option since they call do_fork() directly. So do_fork() enforces a somewhat arbitrary calling convention in the first place that doesn't really help the individual architectures that deviate from it. They can thus simply be switched to _do_fork() enforcing a single calling convention. (I really hope that any new architectures will __not__ try to implement their own calling conventions...) Most architectures already have made a similar switch (m68k comes to mind). Overall this removes more code than it adds even with a good portion of added comments. It simplifies a chunk of arch specific assembly either by moving the code into C or by simply rewriting the assembly. Architectures that have been touched in non-trivial ways have all been actually boot and stress tested: sparc and ia64 have been tested with Debian 9 images. They are the two architectures which have been touched the most. All non-trivial changes to architectures have seen acks from the relevant maintainers. nios2 with a custom built buildroot image. h8300 I couldn't get something bootable to test on but the changes have been fairly automatic and I'm sure we'll hear people yell if I broke something there. All other architectures that have been touched in trivial ways have been compile tested for each single patch of the series via git rebase -x "make ..." v5.8-rc2. arm{64} and x86{_64} have been boot tested even though they have just been trivially touched (removal of the HAVE_COPY_THREAD_TLS macro from their Kconfig) because well they are basically "core architectures" and since it is trivial to get your hands on a useable image" * tag 'fork-v5.9' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux: arch: rename copy_thread_tls() back to copy_thread() arch: remove HAVE_COPY_THREAD_TLS unicore: switch to copy_thread_tls() sh: switch to copy_thread_tls() nds32: switch to copy_thread_tls() microblaze: switch to copy_thread_tls() hexagon: switch to copy_thread_tls() c6x: switch to copy_thread_tls() alpha: switch to copy_thread_tls() fork: remove do_fork() h8300: select HAVE_COPY_THREAD_TLS, switch to kernel_clone_args nios2: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args sparc: unconditionally enable HAVE_COPY_THREAD_TLS sparc: share process creation helpers between sparc and sparc64 sparc64: enable HAVE_COPY_THREAD_TLS fork: fold legacy_clone_args_valid() into _do_fork()
Showing the changes to kernel (1 of 52 files). Show all
Unified · Split
| @@ -2097,8 +2097,7 @@ static __latent_entropy struct task_struct *copy_process( · 2096 unmodified lines | |||
| 2097 | retval = copy_io(clone_flags, p); | 2097 | retval = copy_io(clone_flags, p); |
| 2098 | if (retval) | 2098 | if (retval) |
| 2099 | goto bad_fork_cleanup_namespaces; | 2099 | goto bad_fork_cleanup_namespaces; |
| 2100 | retval = copy_thread_tls(clone_flags, args->stack, args->stack_size, p, | 2100 | retval = copy_thread(clone_flags, args->stack, args->stack_size, p, args->tls); |
| 2101 | args->tls); | ||
| 2102 | if (retval) | 2101 | if (retval) |
| 2103 | goto bad_fork_cleanup_io; | 2102 | goto bad_fork_cleanup_io; |
| 2104 | 2103 | ||
| @@ -2416,6 +2415,20 @@ long _do_fork(struct kernel_clone_args *args) · 311 unmodified lines | |||
| 2416 | int trace = 0; | 2415 | int trace = 0; |
| 2417 | long nr; | 2416 | long nr; |
| 2418 | 2417 | ||
| 2418 | /* | ||
| 2419 | * For legacy clone() calls, CLONE_PIDFD uses the parent_tid argument | ||
| 2420 | * to return the pidfd. Hence, CLONE_PIDFD and CLONE_PARENT_SETTID are | ||
| 2421 | * mutually exclusive. With clone3() CLONE_PIDFD has grown a separate | ||
| 2422 | * field in struct clone_args and it still doesn't make sense to have | ||
| 2423 | * them both point at the same memory location. Performing this check | ||
| 2424 | * here has the advantage that we don't need to have a separate helper | ||
| 2425 | * to check for legacy clone(). | ||
| 2426 | */ | ||
| 2427 | if ((args->flags & CLONE_PIDFD) && | ||
| 2428 | (args->flags & CLONE_PARENT_SETTID) && | ||
| 2429 | (args->pidfd == args->parent_tid)) | ||
| 2430 | return -EINVAL; | ||
| 2431 | |||
| 2419 | /* | 2432 | /* |
| 2420 | * Determine whether and which event to report to ptracer. When | 2433 | * Determine whether and which event to report to ptracer. When |
| 2421 | * called from kernel_thread or CLONE_UNTRACED is explicitly | 2434 | * called from kernel_thread or CLONE_UNTRACED is explicitly |
| @@ -2473,42 +2486,6 @@ long _do_fork(struct kernel_clone_args *args) · 51 unmodified lines | |||
| 2473 | return nr; | 2486 | return nr; |
| 2474 | } | 2487 | } |
| 2475 | 2488 | ||
| 2476 | bool legacy_clone_args_valid(const struct kernel_clone_args *kargs) | ||
| 2477 | { | ||
| 2478 | /* clone(CLONE_PIDFD) uses parent_tidptr to return a pidfd */ | ||
| 2479 | if ((kargs->flags & CLONE_PIDFD) && | ||
| 2480 | (kargs->flags & CLONE_PARENT_SETTID)) | ||
| 2481 | return false; | ||
| 2482 | |||
| 2483 | return true; | ||
| 2484 | } | ||
| 2485 | |||
| 2486 | #ifndef CONFIG_HAVE_COPY_THREAD_TLS | ||
| 2487 | /* For compatibility with architectures that call do_fork directly rather than | ||
| 2488 | * using the syscall entry points below. */ | ||
| 2489 | long do_fork(unsigned long clone_flags, | ||
| 2490 | unsigned long stack_start, | ||
| 2491 | unsigned long stack_size, | ||
| 2492 | int __user *parent_tidptr, | ||
| 2493 | int __user *child_tidptr) | ||
| 2494 | { | ||
| 2495 | struct kernel_clone_args args = { | ||
| 2496 | .flags = (lower_32_bits(clone_flags) & ~CSIGNAL), | ||
| 2497 | .pidfd = parent_tidptr, | ||
| 2498 | .child_tid = child_tidptr, | ||
| 2499 | .parent_tid = parent_tidptr, | ||
| 2500 | .exit_signal = (lower_32_bits(clone_flags) & CSIGNAL), | ||
| 2501 | .stack = stack_start, | ||
| 2502 | .stack_size = stack_size, | ||
| 2503 | }; | ||
| 2504 | |||
| 2505 | if (!legacy_clone_args_valid(&args)) | ||
| 2506 | return -EINVAL; | ||
| 2507 | |||
| 2508 | return _do_fork(&args); | ||
| 2509 | } | ||
| 2510 | #endif | ||
| 2511 | |||
| 2512 | /* | 2489 | /* |
| 2513 | * Create a kernel thread. | 2490 | * Create a kernel thread. |
| 2514 | */ | 2491 | */ |
| @@ -2587,24 +2564,12 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp, · 72 unmodified lines | |||
| 2587 | .tls = tls, | 2564 | .tls = tls, |
| 2588 | }; | 2565 | }; |
| 2589 | 2566 | ||
| 2590 | if (!legacy_clone_args_valid(&args)) | ||
| 2591 | return -EINVAL; | ||
| 2592 | |||
| 2593 | return _do_fork(&args); | 2567 | return _do_fork(&args); |
| 2594 | } | 2568 | } |
| 2595 | #endif | 2569 | #endif |
| 2596 | 2570 | ||
| 2597 | #ifdef __ARCH_WANT_SYS_CLONE3 | 2571 | #ifdef __ARCH_WANT_SYS_CLONE3 |
| 2598 | 2572 | ||
| 2599 | /* | ||
| 2600 | * copy_thread implementations handle CLONE_SETTLS by reading the TLS value from | ||
| 2601 | * the registers containing the syscall arguments for clone. This doesn't work | ||
| 2602 | * with clone3 since the TLS value is passed in clone_args instead. | ||
| 2603 | */ | ||
| 2604 | #ifndef CONFIG_HAVE_COPY_THREAD_TLS | ||
| 2605 | #error clone3 requires copy_thread_tls support in arch | ||
| 2606 | #endif | ||
| 2607 | |||
| 2608 | noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, | 2573 | noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, |
| 2609 | struct clone_args __user *uargs, | 2574 | struct clone_args __user *uargs, |
| 2610 | size_t usize) | 2575 | size_t usize) |
| @@ -2919,7 +2884,7 @@ static int unshare_fd(unsigned long unshare_flags, struct files_struct **new_fdp · 308 unmodified lines | |||
| 2919 | /* | 2884 | /* |
| 2920 | * unshare allows a process to 'unshare' part of the process | 2885 | * unshare allows a process to 'unshare' part of the process |
| 2921 | * context which was originally shared using clone. copy_* | 2886 | * context which was originally shared using clone. copy_* |
| 2922 | * functions used by do_fork() cannot be used here directly | 2887 | * functions used by _do_fork() cannot be used here directly |
| 2923 | * because they modify an inactive task_struct that is being | 2888 | * because they modify an inactive task_struct that is being |
| 2924 | * constructed. Here we are modifying the current, active, | 2889 | * constructed. Here we are modifying the current, active, |
| 2925 | * task_struct. | 2890 | * task_struct. |