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@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Lennart Poettering <mzxreary@0pointer.de>
Cc: Luca Boccassi <bluca@debian.org>
Cc: David Rheinsberg <david@readahead.eu>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.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)
+{
+	/* drop all previous references */
+	scm_destroy_cred(scm);
+
+	scm->pid = get_pid(pid);
+	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

[dropped my previous email address]

On Sun, Jun 29, 2025 at 2:45 PM Alexander Mikhalitsyn
<aleksandr.mikhalitsyn@canonical.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@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Kuniyuki Iwashima <kuniyu@google.com>
> Cc: Lennart Poettering <mzxreary@0pointer.de>
> Cc: Luca Boccassi <bluca@debian.org>
> Cc: David Rheinsberg <david@readahead.eu>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.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 ?).

> +{
> +       /* 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().

> +       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
>

On Mon, Jun 30, 2025 at 9:46 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> [dropped my previous email address]
>
> On Sun, Jun 29, 2025 at 2:45 PM Alexander Mikhalitsyn
> <aleksandr.mikhalitsyn@canonical.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@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Paolo Abeni <pabeni@redhat.com>
> > Cc: Simon Horman <horms@kernel.org>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Leon Romanovsky <leon@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Kuniyuki Iwashima <kuniyu@google.com>
> > Cc: Lennart Poettering <mzxreary@0pointer.de>
> > Cc: Luca Boccassi <bluca@debian.org>
> > Cc: David Rheinsberg <david@readahead.eu>
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.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
> >