net: mctp: mctp_fraq_queue should take ownership of passed skb

[ Upstream commit 773b27a8a2f00ce3134e92e50ea4794a98ba2b76 ]

As of commit f5d83cf0eeb9 ("net: mctp: unshare packets when
reassembling"), we skb_unshare() in mctp_frag_queue(). The unshare may
invalidate the original skb pointer, so we need to treat the skb as
entirely owned by the fraq queue, even on failure.

Fixes: f5d83cf0eeb9 ("net: mctp: unshare packets when reassembling")
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://patch.msgid.link/20250829-mctp-skb-unshare-v1-1-1c28fe10235a@codeconstruct.com.au
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/mctp/route.c b/net/mctp/route.c
index d9c8e5a5f9ce9aefbf16730c65a1f54caa5592b9..19ff259d7bc4379936b5040cb463144beaeda012 100644 (file)
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -325,6 +325,7 @@ static void mctp_skb_set_flow(struct sk_buff *skb, struct mctp_sk_key *key) {}
 static void mctp_flow_prepare_output(struct sk_buff *skb, struct mctp_dev *dev) {}
 #endif
 
+/* takes ownership of skb, both in success and failure cases */
 static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)
 {
        struct mctp_hdr *hdr = mctp_hdr(skb);
@@ -334,8 +335,10 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)
                & MCTP_HDR_SEQ_MASK;
 
        if (!key->reasm_head) {
-               /* Since we're manipulating the shared frag_list, ensure it isn't
-                * shared with any other SKBs.
+               /* Since we're manipulating the shared frag_list, ensure it
+                * isn't shared with any other SKBs. In the cloned case,
+                * this will free the skb; callers can no longer access it
+                * safely.
                 */
                key->reasm_head = skb_unshare(skb, GFP_ATOMIC);
                if (!key->reasm_head)
@@ -349,10 +352,10 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)
        exp_seq = (key->last_seq + 1) & MCTP_HDR_SEQ_MASK;
 
        if (this_seq != exp_seq)
-               return -EINVAL;
+               goto err_free;
 
        if (key->reasm_head->len + skb->len > mctp_message_maxlen)
-               return -EINVAL;
+               goto err_free;
 
        skb->next = NULL;
        skb->sk = NULL;
@@ -366,6 +369,10 @@ static int mctp_frag_queue(struct mctp_sk_key *key, struct sk_buff *skb)
        key->reasm_head->truesize += skb->truesize;
 
        return 0;
+
+err_free:
+       kfree_skb(skb);
+       return -EINVAL;
 }
 
 static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
@@ -476,18 +483,16 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
                         * key isn't observable yet
                         */
                        mctp_frag_queue(key, skb);
+                       skb = NULL;
 
                        /* if the key_add fails, we've raced with another
                         * SOM packet with the same src, dest and tag. There's
                         * no way to distinguish future packets, so all we
-                        * can do is drop; we'll free the skb on exit from
-                        * this function.
+                        * can do is drop.
                         */
                        rc = mctp_key_add(key, msk);
-                       if (!rc) {
+                       if (!rc)
                                trace_mctp_key_acquire(key);
-                               skb = NULL;
-                       }
 
                        /* we don't need to release key->lock on exit, so
                         * clean up here and suppress the unlock via
@@ -505,8 +510,7 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
                                key = NULL;
                        } else {
                                rc = mctp_frag_queue(key, skb);
-                               if (!rc)
-                                       skb = NULL;
+                               skb = NULL;
                        }
                }
 
@@ -516,17 +520,16 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
                 */
 
                /* we need to be continuing an existing reassembly... */
-               if (!key->reasm_head)
+               if (!key->reasm_head) {
                        rc = -EINVAL;
-               else
+               } else {
                        rc = mctp_frag_queue(key, skb);
+                       skb = NULL;
+               }
 
                if (rc)
                        goto out_unlock;
 
-               /* we've queued; the queue owns the skb now */
-               skb = NULL;
-
                /* end of message? deliver to socket, and we're done with
                 * the reassembly/response key
                 */