Commit 773b27a8 authored by Jeremy Kerr's avatar Jeremy Kerr Committed by Paolo Abeni
Browse files

net: mctp: mctp_fraq_queue should take ownership of passed skb



As of commit f5d83cf0 ("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: f5d83cf0 ("net: mctp: unshare packets when reassembling")
Signed-off-by: default avatarJeremy Kerr <jk@codeconstruct.com.au>
Link: https://patch.msgid.link/20250829-mctp-skb-unshare-v1-1-1c28fe10235a@codeconstruct.com.au


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent ba1e9421
Loading
Loading
Loading
Loading
+19 −16
Original line number Diff line number Diff line
@@ -378,6 +378,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);
@@ -387,8 +388,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)
@@ -402,10 +405,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;
@@ -419,6 +422,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_dst_input(struct mctp_dst *dst, struct sk_buff *skb)
@@ -532,18 +539,16 @@ static int mctp_dst_input(struct mctp_dst *dst, 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
@@ -561,7 +566,6 @@ static int mctp_dst_input(struct mctp_dst *dst, struct sk_buff *skb)
				key = NULL;
			} else {
				rc = mctp_frag_queue(key, skb);
				if (!rc)
				skb = NULL;
			}
		}
@@ -572,17 +576,16 @@ static int mctp_dst_input(struct mctp_dst *dst, 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
		 */