rxrpc: Fix potential UAF after skb_unshare() failure

[ Upstream commit 1f2740150f ]

If skb_unshare() fails to unshare a packet due to allocation failure in
rxrpc_input_packet(), the skb pointer in the parent (rxrpc_io_thread())
will be NULL'd out.  This will likely cause the call to
trace_rxrpc_rx_done() to oops.

Fix this by moving the unsharing down to where rxrpc_input_call_event()
calls rxrpc_input_call_packet().  There are a number of places prior to
that where we ignore DATA packets for a variety of reasons (such as the
call already being complete) for which an unshare is then avoided.

And with that, rxrpc_input_packet() doesn't need to take a pointer to the
pointer to the packet, so change that to just a pointer.

Fixes: 2d1faf7a0c ("rxrpc: Simplify skbuff accounting in receive path")
Closes: https://sashiko.dev/#/patchset/20260408121252.2249051-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@kernel.org
Link: https://patch.msgid.link/20260422161438.2593376-4-dhowells@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[ adapted to per-skb rxrpc_input_call_event() signature ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
David Howells
2026-05-03 06:59:34 -04:00
committed by Greg Kroah-Hartman
parent 3a9d8ec205
commit bf20f46d94
5 changed files with 26 additions and 36 deletions
+2 -2
View File
@@ -127,8 +127,7 @@
E_(rxrpc_call_poke_timer_now, "Timer-now")
#define rxrpc_skb_traces \
EM(rxrpc_skb_eaten_by_unshare, "ETN unshare ") \
EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \
EM(rxrpc_skb_get_call_rx, "GET call-rx ") \
EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \
EM(rxrpc_skb_get_conn_work, "GET conn-work") \
EM(rxrpc_skb_get_last_nack, "GET last-nack") \
@@ -153,6 +152,7 @@
EM(rxrpc_skb_see_recvmsg, "SEE recvmsg ") \
EM(rxrpc_skb_see_reject, "SEE reject ") \
EM(rxrpc_skb_see_rotate, "SEE rotate ") \
EM(rxrpc_skb_see_unshare_nomem, "SEE unshar-nm") \
E_(rxrpc_skb_see_version, "SEE version ")
#define rxrpc_local_traces \
-1
View File
@@ -1260,7 +1260,6 @@ int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
void rxrpc_kernel_data_consumed(struct rxrpc_call *, struct sk_buff *);
void rxrpc_new_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_see_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_eaten_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_get_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_free_skb(struct sk_buff *, enum rxrpc_skb_trace);
void rxrpc_purge_queue(struct sk_buff_head *);
+22 -2
View File
@@ -342,8 +342,28 @@ bool rxrpc_input_call_event(struct rxrpc_call *call, struct sk_buff *skb)
if (skb && skb->mark == RXRPC_SKB_MARK_ERROR)
goto out;
if (skb)
rxrpc_input_call_packet(call, skb);
if (skb) {
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
if (sp->hdr.securityIndex != 0 &&
skb_cloned(skb)) {
/* Unshare the packet so that it can be modified for
* in-place decryption.
*/
struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
if (nskb) {
rxrpc_new_skb(nskb, rxrpc_skb_new_unshared);
rxrpc_input_call_packet(call, nskb);
rxrpc_free_skb(nskb, rxrpc_skb_put_input);
} else {
/* OOM - Drop the packet. */
rxrpc_see_skb(skb, rxrpc_skb_see_unshare_nomem);
}
} else {
rxrpc_input_call_packet(call, skb);
}
}
/* If we see our async-event poke, check for timeout trippage. */
now = ktime_get_real();
+2 -22
View File
@@ -178,13 +178,12 @@ static bool rxrpc_extract_abort(struct sk_buff *skb)
/*
* Process packets received on the local endpoint
*/
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff *skb)
{
struct rxrpc_connection *conn;
struct sockaddr_rxrpc peer_srx;
struct rxrpc_skb_priv *sp;
struct rxrpc_peer *peer = NULL;
struct sk_buff *skb = *_skb;
bool ret = false;
skb_pull(skb, sizeof(struct udphdr));
@@ -230,25 +229,6 @@ static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_call);
if (sp->hdr.seq == 0)
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_seq);
/* Unshare the packet so that it can be modified for in-place
* decryption.
*/
if (sp->hdr.securityIndex != 0) {
skb = skb_unshare(skb, GFP_ATOMIC);
if (!skb) {
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare_nomem);
*_skb = NULL;
return just_discard;
}
if (skb != *_skb) {
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare);
*_skb = skb;
rxrpc_new_skb(skb, rxrpc_skb_new_unshared);
sp = rxrpc_skb(skb);
}
}
break;
case RXRPC_PACKET_TYPE_CHALLENGE:
@@ -490,7 +470,7 @@ int rxrpc_io_thread(void *data)
switch (skb->mark) {
case RXRPC_SKB_MARK_PACKET:
skb->priority = 0;
if (!rxrpc_input_packet(local, &skb))
if (!rxrpc_input_packet(local, skb))
rxrpc_reject_packet(local, skb);
trace_rxrpc_rx_done(skb->mark, skb->priority);
rxrpc_free_skb(skb, rxrpc_skb_put_input);
-9
View File
@@ -46,15 +46,6 @@ void rxrpc_get_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
skb_get(skb);
}
/*
* Note the dropping of a ref on a socket buffer by the core.
*/
void rxrpc_eaten_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
{
int n = atomic_inc_return(&rxrpc_n_rx_skbs);
trace_rxrpc_skb(skb, 0, n, why);
}
/*
* Note the destruction of a socket buffer.
*/