Re: [PATCH 09/10] pidfs: record the coredump on the dumping thread's pid too

[Alexander Mikhalitsyn]

Am Mo., 31. Aug. 2026 um 13:22 Uhr schrieb Christian Brauner
<brauner@xxxxxxxxxx>:
>
> If a thread-group coredumps only the thread-group leader pidfd will
> return coredump information. A pidfd for the thread that took the fatal
> signal cannot be used to retrieve it.
>
> Record both the thread-group leader and the specific thread that took
> the signal and register both in pidfs. Mark both the thread-group leader
> and the specific thread with the coredump information so retrieval works
> for both pidfds.
>
> Now that both SO_PEERPIDFD and SO_PEERPIDFD_THREAD are available it's
> easy to get the coredump information for the specific thread.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>

LGTM

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

> ---
>  fs/coredump.c            | 22 +++++++++++++---------
>  fs/pidfs.c               | 11 +++++++++--
>  include/linux/coredump.h |  4 +++-
>  3 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 71a0093ada1b..b5ff4b3e1831 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -454,7 +454,7 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
>                                  * leader we know that the thread-group leader
>                                  * cannot be reaped until @current has exited.
>                                  */
> -                               cprm->pid = task_tgid(current);
> +                               task_pids(cprm->pid, current);
>                                 err = cn_printf(cn, "%d", COREDUMP_PIDFD_NUMBER);
>                                 break;
>                         }
> @@ -626,13 +626,17 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
>         struct coredump_params *cp = (struct coredump_params *)info->data;
>         int err;
>
> -       if (cp->pid) {
> +       if (cp->pid[PIDTYPE_TGID]) {
>                 struct file *pidfs_file __free(fput) = NULL;
>
> -               pidfs_file = pidfs_alloc_file(cp->pid, 0);
> +               pidfs_file = pidfs_alloc_file(cp->pid[PIDTYPE_TGID], 0);
>                 if (IS_ERR(pidfs_file))
>                         return PTR_ERR(pidfs_file);
>
> +               err = pidfs_register_pids(cp->pid);
> +               if (err)
> +                       return err;
> +
>                 pidfs_coredump(cp);
>
>                 /*
> @@ -695,12 +699,12 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
>                 return false;
>
>         /*
> -        * Set the thread-group leader pid which is used for the peer
> -        * credentials during connect() below. Then immediately register
> -        * it in pidfs...
> +        * Set the pids of the dumping thread and its thread-group leader
> +        * which are used for the peer credentials during connect() below.
> +        * Then immediately register them in pidfs...
>          */
> -       cprm->pid = task_tgid(current);
> -       retval = pidfs_register_pid(cprm->pid);
> +       task_pids(cprm->pid, current);
> +       retval = pidfs_register_pids(cprm->pid);
>         if (retval)
>                 return false;
>
> @@ -722,7 +726,7 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
>         }
>
>         /* ... and validate that @sk_peer_pid matches @cprm.pid. */
> -       if (WARN_ON_ONCE(unix_peer(socket->sk)->sk_peer_pid[PIDTYPE_TGID] != cprm->pid))
> +       if (WARN_ON_ONCE(!pids_equal(unix_peer(socket->sk)->sk_peer_pid, cprm->pid)))
>                 return false;
>
>         cprm->limit = RLIM_INFINITY;
> diff --git a/fs/pidfs.c b/fs/pidfs.c
> index 586af2e5811c..29299b2c7ca7 100644
> --- a/fs/pidfs.c
> +++ b/fs/pidfs.c
> @@ -793,9 +793,9 @@ void pidfs_exit(struct task_struct *tsk)
>  }
>
>  #ifdef CONFIG_COREDUMP
> -void pidfs_coredump(const struct coredump_params *cprm)
> +static void pidfs_coredump_pid(struct pid *pid,
> +                              const struct coredump_params *cprm)
>  {
> -       struct pid *pid = cprm->pid;
>         struct pidfs_attr *attr;
>
>         attr = READ_ONCE(pid->attr);
> @@ -814,6 +814,13 @@ void pidfs_coredump(const struct coredump_params *cprm)
>         smp_wmb();
>         set_bit(PIDFS_ATTR_BIT_COREDUMP, &attr->attr_mask);
>  }
> +
> +void pidfs_coredump(const struct coredump_params *cprm)
> +{
> +       /* The dumping thread's pidfd reports the coredump as well. */
> +       for (enum pid_type type = PIDTYPE_PID; type <= pids_last(cprm->pid); type++)
> +               pidfs_coredump_pid(cprm->pid[type], cprm);
> +}
>  #endif
>
>  static struct vfsmount *pidfs_mnt __ro_after_init;
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index 7b38ee2e7913..0bbb7de6a402 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -5,6 +5,7 @@
>  #include <linux/types.h>
>  #include <linux/mm.h>
>  #include <linux/fs.h>
> +#include <linux/pid_types.h>
>  #include <linux/sched/coredump.h>
>  #include <asm/siginfo.h>
>
> @@ -32,7 +33,8 @@ struct coredump_params {
>         int vma_count;
>         size_t vma_data_size;
>         struct core_vma_metadata *vma_meta;
> -       struct pid *pid;
> +       /* Dumping thread and its thread-group leader by pid type. */
> +       DECLARE_PIDS(pid, PIDTYPE_TGID);
>  };
>
>  extern unsigned int core_file_note_size_limit;
>
> --
> 2.53.0
>