Re: [PATCH 02/10] af_unix: record the pid of the sending thread

[Alexander Mikhalitsyn]

Am Mi., 9. Sept. 2026 um 12:36 Uhr schrieb Christian Brauner
<brauner@xxxxxxxxxx>:
>
> On Mon, Sep 07, 2026 at 11:55:23AM +0200, Alexander Mikhalitsyn wrote:
> > Am Mo., 31. Aug. 2026 um 13:21 Uhr schrieb Christian Brauner
> > <brauner@xxxxxxxxxx>:
> > >
> > > Currently only the struct pid of the thread-group leader is recorded.
> > > The identity of the actual thread that sent the message or is connected
> > > to a given socket cannot be retrieved.
> > >
> > > Add the plumbing to make it possible to retrieve a pidfd for the sender.
> > > Nothing uses the thread-specific struct pid yet. No functional changes.
> > >
> > > Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>
> > > ---
> > >  include/net/scm.h  |  7 +++----
> > >  net/core/scm.c     | 21 +++++++++++++++++----
> > >  net/unix/af_unix.c | 23 +++++++++++++----------
> > >  net/unix/af_unix.h |  3 ++-
> > >  4 files changed, 35 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/include/net/scm.h b/include/net/scm.h
> > > index 86ae6bc109ec..aa7d15c5fc27 100644
> > > --- a/include/net/scm.h
> > > +++ b/include/net/scm.h
> > > @@ -42,7 +42,7 @@ struct scm_fp_list {
> > >  };
> > >
> > >  struct scm_cookie {
> > > -       struct pid              *pid;           /* Skb credentials */
> > > +       DECLARE_PIDS(pid, PIDTYPE_TGID);        /* Skb credentials by pid type */
> > >         struct scm_fp_list      *fp;            /* Passed files         */
> > >         struct scm_creds        creds;          /* Skb credentials      */
> > >  #ifdef CONFIG_SECURITY_NETWORK
> > > @@ -69,7 +69,7 @@ static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_co
> > >  static __inline__ void scm_set_cred(struct scm_cookie *scm,
> > >                                     struct pid *pid, kuid_t uid, kgid_t gid)
> > >  {
> > > -       scm->pid = get_pid(pid);
> > > +       scm->pid[PIDTYPE_TGID] = get_pid(pid);
> > >         scm->creds.pid = pid_vnr(pid);
> > >         scm->creds.uid = uid;
> > >         scm->creds.gid = gid;
> > > @@ -77,8 +77,7 @@ static __inline__ void scm_set_cred(struct scm_cookie *scm,
> > >
> > >  static __inline__ void scm_destroy_cred(struct scm_cookie *scm)
> > >  {
> > > -       put_pid(scm->pid);
> > > -       scm->pid = NULL;
> > > +       put_pids(scm->pid);
> > >  }
> > >
> > >  static __inline__ void scm_destroy(struct scm_cookie *scm)
> > > diff --git a/net/core/scm.c b/net/core/scm.c
> > > index f0d44ecdb11f..9b9e119c353a 100644
> > > --- a/net/core/scm.c
> > > +++ b/net/core/scm.c
> > > @@ -149,6 +149,7 @@ EXPORT_SYMBOL(__scm_destroy);
> > >
> > >  static inline int scm_replace_pid(struct scm_cookie *scm, struct pid *pid)
> > >  {
> > > +       struct pid *thread_pid;
> > >         int err;
> > >
> > >         /* drop all previous references */
> > > @@ -158,7 +159,18 @@ static inline int scm_replace_pid(struct scm_cookie *scm, struct pid *pid)
> > >         if (unlikely(err))
> > >                 return err;
> > >
> > > -       scm->pid = pid;
> > > +       /* A sender naming its own thread-group sends from the current thread. */
> > > +       if (pid == task_tgid(current))
> > > +               thread_pid = task_pid(current);
> > > +       else
> > > +               thread_pid = pid;
> >
> > Hi Christian,
> >
> > The patch looks good to me, but this spot raised some doubts for me.
> >
> > If scm_replace_pid() is called with pid == task_tgid(current) then all
> > good, because we
> > get scm->pid[PIDTYPE_TGID] == pid and  scm->pid[PIDTYPE_PID] ==
> > task_pid(current). All good.
> >
> > If scm_replace_pid() is supplied with pid that has a type PIDTYPE_TGID, then:
> > - scm->pid[PIDTYPE_TGID] == pid. Good.
> > - scm->pid[PIDTYPE_PID] = pid. Doesn't looks correct, shoudn't we do
>
> After chatting with you, I think I get the concern. You think that
> scm->pid[PIDTYPE_PID] always points to a thread-group leader if pid ==
> task_tgid(current). Yes, that's true but that's intended and fine. The
> thread-group leader is just a thread in the thread-group just with
> magical behavior (that we would really like to get rid of but can't
> easily...).

Yes, my concern was about another case when `pid != task_tgid(current)`, but the
chunk of code I've suggested makes no sense. When I looked at it with
a fresh head I realized this.
In scm_replace_pid, once we've got `pid` pointing to a leader's thread
we can't really do anything
other than taking leader thread's TID as a scm->pid[PIDTYPE_PID]...

>
> Thanks for looking closely at this though!

LGTM

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