Skip to content

Commit 1f27401

Browse files
dhowellskuba-moo
authored andcommitted
rxrpc: Fix potential UAF after skb_unshare() failure
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: 2d1faf7 ("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>
1 parent def304a commit 1f27401

5 files changed

Lines changed: 22 additions & 35 deletions

File tree

include/trace/events/rxrpc.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,6 @@
162162
E_(rxrpc_call_poke_timer_now, "Timer-now")
163163

164164
#define rxrpc_skb_traces \
165-
EM(rxrpc_skb_eaten_by_unshare, "ETN unshare ") \
166-
EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \
167165
EM(rxrpc_skb_get_call_rx, "GET call-rx ") \
168166
EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \
169167
EM(rxrpc_skb_get_conn_work, "GET conn-work") \
@@ -190,6 +188,7 @@
190188
EM(rxrpc_skb_put_purge, "PUT purge ") \
191189
EM(rxrpc_skb_put_purge_oob, "PUT purge-oob") \
192190
EM(rxrpc_skb_put_response, "PUT response ") \
191+
EM(rxrpc_skb_put_response_copy, "PUT resp-cpy ") \
193192
EM(rxrpc_skb_put_rotate, "PUT rotate ") \
194193
EM(rxrpc_skb_put_unknown, "PUT unknown ") \
195194
EM(rxrpc_skb_see_conn_work, "SEE conn-work") \
@@ -198,6 +197,7 @@
198197
EM(rxrpc_skb_see_recvmsg_oob, "SEE recvm-oob") \
199198
EM(rxrpc_skb_see_reject, "SEE reject ") \
200199
EM(rxrpc_skb_see_rotate, "SEE rotate ") \
200+
EM(rxrpc_skb_see_unshare_nomem, "SEE unshar-nm") \
201201
E_(rxrpc_skb_see_version, "SEE version ")
202202

203203
#define rxrpc_local_traces \

net/rxrpc/ar-internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1486,7 +1486,6 @@ int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
14861486
void rxrpc_kernel_data_consumed(struct rxrpc_call *, struct sk_buff *);
14871487
void rxrpc_new_skb(struct sk_buff *, enum rxrpc_skb_trace);
14881488
void rxrpc_see_skb(struct sk_buff *, enum rxrpc_skb_trace);
1489-
void rxrpc_eaten_skb(struct sk_buff *, enum rxrpc_skb_trace);
14901489
void rxrpc_get_skb(struct sk_buff *, enum rxrpc_skb_trace);
14911490
void rxrpc_free_skb(struct sk_buff *, enum rxrpc_skb_trace);
14921491
void rxrpc_purge_queue(struct sk_buff_head *);

net/rxrpc/call_event.c

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,24 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
332332

333333
saw_ack |= sp->hdr.type == RXRPC_PACKET_TYPE_ACK;
334334

335-
rxrpc_input_call_packet(call, skb);
335+
if (sp->hdr.securityIndex != 0 &&
336+
skb_cloned(skb)) {
337+
/* Unshare the packet so that it can be
338+
* modified by in-place decryption.
339+
*/
340+
struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
341+
342+
if (nskb) {
343+
rxrpc_new_skb(nskb, rxrpc_skb_new_unshared);
344+
rxrpc_input_call_packet(call, nskb);
345+
rxrpc_free_skb(nskb, rxrpc_skb_put_call_rx);
346+
} else {
347+
/* OOM - Drop the packet. */
348+
rxrpc_see_skb(skb, rxrpc_skb_see_unshare_nomem);
349+
}
350+
} else {
351+
rxrpc_input_call_packet(call, skb);
352+
}
336353
rxrpc_free_skb(skb, rxrpc_skb_put_call_rx);
337354
did_receive = true;
338355
}

net/rxrpc/io_thread.c

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,12 @@ static bool rxrpc_extract_abort(struct sk_buff *skb)
192192
/*
193193
* Process packets received on the local endpoint
194194
*/
195-
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
195+
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff *skb)
196196
{
197197
struct rxrpc_connection *conn;
198198
struct sockaddr_rxrpc peer_srx;
199199
struct rxrpc_skb_priv *sp;
200200
struct rxrpc_peer *peer = NULL;
201-
struct sk_buff *skb = *_skb;
202201
bool ret = false;
203202

204203
skb_pull(skb, sizeof(struct udphdr));
@@ -244,25 +243,6 @@ static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
244243
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_call);
245244
if (sp->hdr.seq == 0)
246245
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_seq);
247-
248-
/* Unshare the packet so that it can be modified for in-place
249-
* decryption.
250-
*/
251-
if (sp->hdr.securityIndex != 0) {
252-
skb = skb_unshare(skb, GFP_ATOMIC);
253-
if (!skb) {
254-
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare_nomem);
255-
*_skb = NULL;
256-
return just_discard;
257-
}
258-
259-
if (skb != *_skb) {
260-
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare);
261-
*_skb = skb;
262-
rxrpc_new_skb(skb, rxrpc_skb_new_unshared);
263-
sp = rxrpc_skb(skb);
264-
}
265-
}
266246
break;
267247

268248
case RXRPC_PACKET_TYPE_CHALLENGE:
@@ -494,7 +474,7 @@ int rxrpc_io_thread(void *data)
494474
switch (skb->mark) {
495475
case RXRPC_SKB_MARK_PACKET:
496476
skb->priority = 0;
497-
if (!rxrpc_input_packet(local, &skb))
477+
if (!rxrpc_input_packet(local, skb))
498478
rxrpc_reject_packet(local, skb);
499479
trace_rxrpc_rx_done(skb->mark, skb->priority);
500480
rxrpc_free_skb(skb, rxrpc_skb_put_input);

net/rxrpc/skbuff.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,6 @@ void rxrpc_get_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
4646
skb_get(skb);
4747
}
4848

49-
/*
50-
* Note the dropping of a ref on a socket buffer by the core.
51-
*/
52-
void rxrpc_eaten_skb(struct sk_buff *skb, enum rxrpc_skb_trace why)
53-
{
54-
int n = atomic_inc_return(&rxrpc_n_rx_skbs);
55-
trace_rxrpc_skb(skb, 0, n, why);
56-
}
57-
5849
/*
5950
* Note the destruction of a socket buffer.
6051
*/

0 commit comments

Comments
 (0)