linux-kernel - Re: [RESEND PATCH net-next 3/6] af_unix: introduce and use __scm_replace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEivzxdEWRsP83X3PLNtLXMZMWGdXaPb8YtduB=cQcXE55e7Mg@mail.gmail.com>
Date: Tue, 1 Jul 2025 10:48:45 +0200
From: Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Willem de Bruijn <willemb@...gle.com>, Leon Romanovsky <leon@...nel.org>, Arnd Bergmann <arnd@...db.de>, 
	Christian Brauner <brauner@...nel.org>, Lennart Poettering <mzxreary@...inter.de>, 
	Luca Boccassi <bluca@...ian.org>, David Rheinsberg <david@...dahead.eu>
Subject: Re: [RESEND PATCH net-next 3/6] af_unix: introduce and use
 __scm_replace_pid() helper

On Mon, Jun 30, 2025 at 9:46 PM Kuniyuki Iwashima <kuniyu@...gle.com> wrote:
>
> [dropped my previous email address]
>
> On Sun, Jun 29, 2025 at 2:45 PM Alexander Mikhalitsyn
> <aleksandr.mikhalitsyn@...onical.com> wrote:
> >
> > Existing logic in __scm_send() related to filling an struct scm_cookie
> > with a proper struct pid reference is already pretty tricky. Let's
> > simplify it a bit by introducing a new helper. This helper will be
> > extended in one of the next patches.
> >
> > Cc: linux-kernel@...r.kernel.org
> > Cc: netdev@...r.kernel.org
> > Cc: "David S. Miller" <davem@...emloft.net>
> > Cc: Eric Dumazet <edumazet@...gle.com>
> > Cc: Jakub Kicinski <kuba@...nel.org>
> > Cc: Paolo Abeni <pabeni@...hat.com>
> > Cc: Simon Horman <horms@...nel.org>
> > Cc: Willem de Bruijn <willemb@...gle.com>
> > Cc: Leon Romanovsky <leon@...nel.org>
> > Cc: Arnd Bergmann <arnd@...db.de>
> > Cc: Christian Brauner <brauner@...nel.org>
> > Cc: Kuniyuki Iwashima <kuniyu@...gle.com>
> > Cc: Lennart Poettering <mzxreary@...inter.de>
> > Cc: Luca Boccassi <bluca@...ian.org>
> > Cc: David Rheinsberg <david@...dahead.eu>
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
> > ---
> >  include/net/scm.h | 10 ++++++++++
> >  net/core/scm.c    | 11 ++++++++---
> >  2 files changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/scm.h b/include/net/scm.h
> > index 84c4707e78a5..856eb3a380f6 100644
> > --- a/include/net/scm.h
> > +++ b/include/net/scm.h
> > @@ -88,6 +88,16 @@ static __inline__ void scm_destroy(struct scm_cookie *scm)
> >                 __scm_destroy(scm);
> >  }
> >
> > +static __inline__ int __scm_replace_pid(struct scm_cookie *scm, struct pid *pid)
>
> It seems this function is only called from __scm_send() so this should
> be moved to .c (and inlined ?).

sure!

>
> > +{
> > +       /* drop all previous references */
> > +       scm_destroy_cred(scm);
> > +
> > +       scm->pid = get_pid(pid);
>
> This looks redundant.  Maybe move the put_pid() under if (error)
> in __scm_send().

yep, fixed in v2.

>
> > +       scm->creds.pid = pid_vnr(pid);
> > +       return 0;
> > +}
> > +
> >  static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
> >                                struct scm_cookie *scm, bool forcecreds)
> >  {
> > diff --git a/net/core/scm.c b/net/core/scm.c
> > index 0225bd94170f..0e71d5a249a1 100644
> > --- a/net/core/scm.c
> > +++ b/net/core/scm.c
> > @@ -189,15 +189,20 @@ int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
> >                         if (err)
> >                                 goto error;
> >
> > -                       p->creds.pid = creds.pid;
> >                         if (!p->pid || pid_vnr(p->pid) != creds.pid) {
> >                                 struct pid *pid;
> >                                 err = -ESRCH;
> >                                 pid = find_get_pid(creds.pid);
> >                                 if (!pid)
> >                                         goto error;
> > -                               put_pid(p->pid);
> > -                               p->pid = pid;
> > +
> > +                               err = __scm_replace_pid(p, pid);
> > +                               /* Release what we get from find_get_pid() as
> > +                                * __scm_replace_pid() takes all necessary refcounts.
> > +                                */
> > +                               put_pid(pid);
> > +                               if (err)
> > +                                       goto error;
> >                         }
> >
> >                         err = -EINVAL;
> > --
> > 2.43.0
> >