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

__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@vger.kernel.org
Assisted-by: Claude:claude-opus-5 codeql
Signed-off-by: Maoyi Xie <redacted>
---
 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);

                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