Re: [PATCH net] netlink: render SCM_CREDENTIALS pid in the receiver's pid namespace

[Eric Dumazet]

On Fri, Sep 4, 2026 at 4:46 PM Maoyi Xie <maoyixie.tju@xxxxxxxxx> wrote:
>
> __scm_recv_common() translates uid and gid into the reader's user
> namespace but copies the pid as is. AF_UNIX gets away with that because
> unix_skb_to_scm() re-renders the pid with pid_vnr() at recvmsg time.
> netlink_sendmsg() renders it in the sender's namespace and stores a bare
> u32, so a reader in another pid namespace sees a number from a namespace
> it is not in. The sender chooses that number. An unprivileged sender in a
> child namespace made the receiver see pid 300.
>
> Carry the sender's struct pid in NETLINK_CB and hand it to scm_set_cred()
> in netlink_recvmsg(), the way af_unix does. netlink_skb_set_owner_r()
> takes the reference and netlink_skb_destructor() drops it. A reader in a
> namespace the sender has no pid in now gets 0, like AF_UNIX.
>
> I found this with a CodeQL checker. I used Claude to help write the
> reproducers. The ones that reproduce the bug run unprivileged and need no
> kernel changes. Tested on net with KASAN and lockdep, no reports. The tree
> has no netlink SCM selftest.
>
> Fixes: b488893a390e ("pid namespaces: changes to show virtual ids to user")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5 codeql
> Signed-off-by: Maoyi Xie <maoyixie.tju@xxxxxxxxx>
> ---
>  include/linux/netlink.h  |  9 +++++++++
>  net/netlink/af_netlink.c | 30 ++++++++++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netlink.h b/include/linux/netlink.h
> index 882e9c1b6c1dcc..26266754e27c14 100644
> --- a/include/linux/netlink.h
> +++ b/include/linux/netlink.h
> @@ -30,6 +30,15 @@ struct netlink_skb_parms {
>         struct sock             *sk;
>         bool                    nsid_is_set;
>         int                     nsid;
> +       /*
> +        * Sender's struct pid.  netlink_sendmsg() stores a borrowed pointer
> +        * taken from its own scm_cookie.  netlink_skb_set_owner_r() takes a
> +        * reference when it takes ownership of the skb for a receiver, and
> +        * netlink_skb_destructor() drops that reference.  A clone starts out
> +        * borrowing again, because __skb_clone() clears both skb->sk and
> +        * skb->destructor.  NULL for a kernel generated skb.
> +        */
> +       struct pid              *pid;
>  };
>
>  #define NETLINK_CB(skb)                (*(struct netlink_skb_parms*)&((skb)->cb))
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index e6b1d9758c9c92..170d90d472a0db 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -379,13 +379,24 @@ static void netlink_skb_destructor(struct sk_buff *skb)
>
>                 skb->head = NULL;
>         }
> -       if (skb->sk != NULL)
> +       if (skb->sk) {
> +               /*
> +                * The reference is held for as long as skb->sk is set, taken
> +                * in netlink_skb_set_owner_r() and dropped here.  The pointer
> +                * is left in place: do_one_broadcast() orphans an skb one
> +                * listener owned and hands it to the next, which takes its
> +                * own reference, and the sender's scm_cookie keeps the pid
> +                * alive across the whole broadcast.
> +                */
> +               put_pid(NETLINK_CB(skb).pid);

I really have a bad feeling about this dance and these assumptions,
maybe this is just me.

Please include a dedicated selftest in tools/testing/selftests/net/
exercising SO_PASSCRED / SCM_CREDENTIALS
over netlink across PID namespaces to demonstrate the issue and
validate the proposed changes.

Thanks.

>                 sock_rfree(skb);
> +       }
>  }
>
>  static void netlink_skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
>  {
>         WARN_ON(skb->sk != NULL);
> +       NETLINK_CB(skb).pid = get_pid(NETLINK_CB(skb).pid);
>         skb->sk = sk;
>         skb->destructor = netlink_skb_destructor;
>         sk_mem_charge(sk, skb->truesize);
> @@ -1880,6 +1891,13 @@ static int netlink_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
>         NETLINK_CB(skb).dst_group = dst_group;
>         NETLINK_CB(skb).creds   = scm.creds;
>         NETLINK_CB(skb).flags   = netlink_skb_flags;
> +       /*
> +        * Borrowed here.  scm_destroy() below drops the scm_cookie's own
> +        * reference, and every delivery in between is synchronous, so the
> +        * pointer stays valid until netlink_skb_set_owner_r() takes a
> +        * reference of its own.
> +        */
> +       NETLINK_CB(skb).pid     = scm.pid;
>
>         err = -EFAULT;
>         if (memcpy_from_msg(skb_put(skb, len), msg, len)) {
> @@ -1971,7 +1989,15 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>                 netlink_cmsg_listen_all_nsid(sk, msg, skb);
>
>         memset(&scm, 0, sizeof(scm));
> -       scm.creds = *NETLINK_CREDS(skb);
> +       /*
> +        * Render the sender's pid in the reader's pid namespace, the way
> +        * unix_skb_to_scm() does through scm_set_cred().  A NULL pid gives 0,
> +        * so a control block that lost its reference reports "unknown" rather
> +        * than the sender's own untranslated number.  scm_recv() below drops
> +        * the reference taken here on both of its paths.
> +        */
> +       scm_set_cred(&scm, NETLINK_CB(skb).pid, NETLINK_CREDS(skb)->uid,
> +                    NETLINK_CREDS(skb)->gid);
>         if (flags & MSG_TRUNC)
>                 copied = data_skb->len;
>
> --
> 2.34.1
>