[<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
> >
Powered by blists - more mailing lists