[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJqdLrrcEwF1s0uLm-z=2DhkmtYLjqwttNujuQ3vT83m-PYLoQ@mail.gmail.com>
Date: Fri, 9 May 2025 18:30:51 +0200
From: Alexander Mikhalitsyn <alexander@...alicyn.com>
To: Christian Brauner <brauner@...nel.org>
Cc: linux-fsdevel@...r.kernel.org, Jann Horn <jannh@...gle.com>,
Daniel Borkmann <daniel@...earbox.net>, Kuniyuki Iwashima <kuniyu@...zon.com>,
Eric Dumazet <edumazet@...gle.com>, Oleg Nesterov <oleg@...hat.com>,
"David S. Miller" <davem@...emloft.net>, Alexander Viro <viro@...iv.linux.org.uk>,
Daan De Meyer <daan.j.demeyer@...il.com>, David Rheinsberg <david@...dahead.eu>,
Jakub Kicinski <kuba@...nel.org>, Jan Kara <jack@...e.cz>,
Lennart Poettering <lennart@...ttering.net>, Luca Boccassi <bluca@...ian.org>, Mike Yuan <me@...dnzj.com>,
Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
Zbigniew Jędrzejewski-Szmek <zbyszek@...waw.pl>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
linux-security-module@...r.kernel.org
Subject: Re: [PATCH v5 2/9] coredump: massage do_coredump()
Am Fr., 9. Mai 2025 um 12:26 Uhr schrieb Christian Brauner <brauner@...nel.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@...nel.org>
Reviewed-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
> ---
> fs/coredump.c | 122 +++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 65 insertions(+), 57 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 281320ea351f..41491dbfafdf 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
>
Powered by blists - more mailing lists