Re: [PATCH v7 2/9] coredump: massage do_coredump()

Alexander Mikhalitsyn
2025-05-15T13:21:51Z

Am Do., 15. Mai 2025 um 00:04 Uhr schrieb Christian Brauner
<brauner@kernel.org>:
>
> We're going to extend the coredump code in follow-up patches.
> Clean it up so we can do this more easily.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>

> ---
> fs/coredump.c | 122 +++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 65 insertions(+), 57 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 368751d98781..0e97c21b35e3 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -646,63 +646,8 @@ void do_coredump(const kernel_siginfo_t *siginfo)
> goto fail_unlock;
> }
>
> - if (cn.core_type == COREDUMP_PIPE) {
> - int argi;
> - int dump_count;
> - char **helper_argv;
> - struct subprocess_info *sub_info;
> -
> - if (cprm.limit == 1) {
> - /* See umh_coredump_setup() which sets RLIMIT_CORE = 1.
> - *
> - * Normally core limits are irrelevant to pipes, since
> - * we're not writing to the file system, but we use
> - * cprm.limit of 1 here as a special value, this is a
> - * consistent way to catch recursive crashes.
> - * We can still crash if the core_pattern binary sets
> - * RLIM_CORE = !1, but it runs as root, and can do
> - * lots of stupid things.
> - *
> - * Note that we use task_tgid_vnr here to grab the pid
> - * of the process group leader. That way we get the
> - * right pid if a thread in a multi-threaded
> - * core_pattern process dies.
> - */
> - coredump_report_failure("RLIMIT_CORE is set to 1, aborting core");
> - goto fail_unlock;
> - }
> - cprm.limit = RLIM_INFINITY;
> -
> - dump_count = atomic_inc_return(&core_dump_count);
> - if (core_pipe_limit && (core_pipe_limit < dump_count)) {
> - coredump_report_failure("over core_pipe_limit, skipping core dump");
> - goto fail_dropcount;
> - }
> -
> - helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv),
> - GFP_KERNEL);
> - if (!helper_argv) {
> - coredump_report_failure("%s failed to allocate memory", __func__);
> - goto fail_dropcount;
> - }
> - for (argi = 0; argi < argc; argi++)
> - helper_argv[argi] = cn.corename + argv[argi];
> - helper_argv[argi] = NULL;
> -
> - retval = -ENOMEM;
> - sub_info = call_usermodehelper_setup(helper_argv[0],
> - helper_argv, NULL, GFP_KERNEL,
> - umh_coredump_setup, NULL, &cprm);
> - if (sub_info)
> - retval = call_usermodehelper_exec(sub_info,
> - UMH_WAIT_EXEC);
> -
> - kfree(helper_argv);
> - if (retval) {
> - coredump_report_failure("|%s pipe failed", cn.corename);
> - goto close_fail;
> - }
> - } else if (cn.core_type == COREDUMP_FILE) {
> + switch (cn.core_type) {
> + case COREDUMP_FILE: {
> struct mnt_idmap *idmap;
> struct inode *inode;
> int open_flags = O_CREAT | O_WRONLY | O_NOFOLLOW |
> @@ -796,6 +741,69 @@ void do_coredump(const kernel_siginfo_t *siginfo)
> if (do_truncate(idmap, cprm.file->f_path.dentry,
> 0, 0, cprm.file))
> goto close_fail;
> + break;
> + }
> + case COREDUMP_PIPE: {
> + int argi;
> + int dump_count;
> + char **helper_argv;
> + struct subprocess_info *sub_info;
> +
> + if (cprm.limit == 1) {
> + /* See umh_coredump_setup() which sets RLIMIT_CORE = 1.
> + *
> + * Normally core limits are irrelevant to pipes, since
> + * we're not writing to the file system, but we use
> + * cprm.limit of 1 here as a special value, this is a
> + * consistent way to catch recursive crashes.
> + * We can still crash if the core_pattern binary sets
> + * RLIM_CORE = !1, but it runs as root, and can do
> + * lots of stupid things.
> + *
> + * Note that we use task_tgid_vnr here to grab the pid
> + * of the process group leader. That way we get the
> + * right pid if a thread in a multi-threaded
> + * core_pattern process dies.
> + */
> + coredump_report_failure("RLIMIT_CORE is set to 1, aborting core");
> + goto fail_unlock;
> + }
> + cprm.limit = RLIM_INFINITY;
> +
> + dump_count = atomic_inc_return(&core_dump_count);
> + if (core_pipe_limit && (core_pipe_limit < dump_count)) {
> + coredump_report_failure("over core_pipe_limit, skipping core dump");
> + goto fail_dropcount;
> + }
> +
> + helper_argv = kmalloc_array(argc + 1, sizeof(*helper_argv),
> + GFP_KERNEL);
> + if (!helper_argv) {
> + coredump_report_failure("%s failed to allocate memory", __func__);
> + goto fail_dropcount;
> + }
> + for (argi = 0; argi < argc; argi++)
> + helper_argv[argi] = cn.corename + argv[argi];
> + helper_argv[argi] = NULL;
> +
> + retval = -ENOMEM;
> + sub_info = call_usermodehelper_setup(helper_argv[0],
> + helper_argv, NULL, GFP_KERNEL,
> + umh_coredump_setup, NULL, &cprm);
> + if (sub_info)
> + retval = call_usermodehelper_exec(sub_info,
> + UMH_WAIT_EXEC);
> +
> + kfree(helper_argv);
> + if (retval) {
> + coredump_report_failure("|%s pipe failed", cn.corename);
> + goto close_fail;
> + }
> + break;
> + }
> + default:
> + WARN_ON_ONCE(true);
> + goto close_fail;
> }
>
> /* get us an unshared descriptor table; almost always a no-op */
>
> --
> 2.47.2
>