hamza.kesraoui/sst-linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

net: mctp: handle skb cleanup on sock_queue failures

Browse Source 
[ Upstream commit ce1219c3f76bb131d095e90521506d3c6ccfa086 ]

Currently, we don't use the return value from sock_queue_rcv_skb, which
means we may leak skbs if a message is not successfully queued to a
socket.

Instead, ensure that we're freeing the skb where the sock hasn't
otherwise taken ownership of the skb by adding checks on the
sock_queue_rcv_skb() to invoke a kfree on failure.

In doing so, rather than using the 'rc' value to trigger the
kfree_skb(), use the skb pointer itself, which is more explicit.

Also, add a kunit test for the sock delivery failure cases.

Fixes: 4a992bbd36 ("mctp: Implement message fragmentation & reassembly")
Cc: stable@vger.kernel.org
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://patch.msgid.link/20241218-mctp-next-v2-1-1c1729645eaa@codeconstruct.com.au
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Jeremy Kerr 2024-12-18 11:53:01 +08:00 committed by Greg Kroah-Hartman
parent 116b937eb4
commit 6c6f477f64
1 changed files with 26 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
net/mctp/route.c
View File

@ -334,8 +334,13 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
msk = NULL; msk = NULL;
rc = -EINVAL; rc = -EINVAL;
/* we may be receiving a locally-routed packet; drop source sk /* We may be receiving a locally-routed packet; drop source sk
* accounting * accounting.
*
* From here, we will either queue the skb - either to a frag_queue, or
* to a receiving socket. When that succeeds, we clear the skb pointer;
* a non-NULL skb on exit will be otherwise unowned, and hence
* kfree_skb()-ed.
*/ */
skb_orphan(skb); skb_orphan(skb);
@ -389,7 +394,9 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* pending key. * pending key.
*/ */
if (flags & MCTP_HDR_FLAG_EOM) { if (flags & MCTP_HDR_FLAG_EOM) {
sock_queue_rcv_skb(&msk->sk, skb); rc = sock_queue_rcv_skb(&msk->sk, skb);
if (!rc)
skb = NULL;
if (key) { if (key) {
/* we've hit a pending reassembly; not much we /* we've hit a pending reassembly; not much we
* can do but drop it * can do but drop it
@ -398,7 +405,6 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
MCTP_TRACE_KEY_REPLIED); MCTP_TRACE_KEY_REPLIED);
key = NULL; key = NULL;
} }
rc = 0;
goto out_unlock; goto out_unlock;
} }
@ -425,8 +431,10 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
* this function. * this function.
*/ */
rc = mctp_key_add(key, msk); rc = mctp_key_add(key, msk);
if (!rc) if (!rc) {
trace_mctp_key_acquire(key); trace_mctp_key_acquire(key);
skb = NULL;
}
/* we don't need to release key->lock on exit, so /* we don't need to release key->lock on exit, so
* clean up here and suppress the unlock via * clean up here and suppress the unlock via
@ -444,6 +452,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
key = NULL; key = NULL;
} else { } else {
rc = mctp_frag_queue(key, skb); rc = mctp_frag_queue(key, skb);
if (!rc)
skb = NULL;
} }
} }
@ -458,12 +468,19 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
else else
rc = mctp_frag_queue(key, skb); rc = mctp_frag_queue(key, skb);
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 /* end of message? deliver to socket, and we're done with
* the reassembly/response key * the reassembly/response key
*/ */
if (!rc && flags & MCTP_HDR_FLAG_EOM) { if (flags & MCTP_HDR_FLAG_EOM) {
sock_queue_rcv_skb(key->sk, key->reasm_head); rc = sock_queue_rcv_skb(key->sk, key->reasm_head);
key->reasm_head = NULL; if (!rc)
key->reasm_head = NULL;
__mctp_key_done_in(key, net, f, MCTP_TRACE_KEY_REPLIED); __mctp_key_done_in(key, net, f, MCTP_TRACE_KEY_REPLIED);
key = NULL; key = NULL;
} }
@ -482,8 +499,7 @@ out_unlock:
if (any_key) if (any_key)
mctp_key_unref(any_key); mctp_key_unref(any_key);
out: out:
if (rc) kfree_skb(skb);
kfree_skb(skb);
return rc; return rc;
} }