Skip to content

Commit f342eb2

Browse files
kerneltoastPlaidCat
authored andcommitted
rxrpc: Fix potential UAF after skb_unshare() failure
cve-pre CVE-2026-43500 commit-author David Howells <dhowells@redhat.com> commit 1f27401 upstream-diff | Upstream's rxrpc_input_call_event() pulls skbs from a per-call queue in a loop, so the unshare block sits inside that loop alongside rxrpc_free_skb() and saw_ack bookkeeping. This kernel's version receives a single skb as a function parameter and doesn't free it (the caller in rxrpc_io_thread() does). As such, the adapted unshare block wraps the existing rxrpc_input_call_packet() call site, creates and frees a copy when needed, and drops the loop and queue-drain logic that doesn't exist here. Upstream already had rxrpc_skb_put_call_rx and rxrpc_skb_get_call_rx trace constants from a prior refactor not present in this tree. rxrpc_skb_put_call_rx is added here since the new skb_copy path needs it; rxrpc_skb_get_call_rx is omitted since nothing uses it. Several trace constants added by upstream (rxrpc_skb_put_purge_oob, rxrpc_skb_put_response, rxrpc_skb_put_response_copy) don't apply since the code they trace doesn't exist in this kernel. Note, Conflict caused by 153f90a ("rxrpc: Use ktimes for call timeout tracking and set the timer lazily"). 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> (cherry picked from commit 1f27401) Signed-off-by: Sultan Alsawaf <sultan@ciq.com> Signed-off-by: Shreeya Patel <spatel@ciq.com> Signed-off-by: Shreeya Patel <spatel@ciq.com>
1 parent 1521408 commit f342eb2

5 files changed

Lines changed: 25 additions & 36 deletions

File tree

include/trace/events/rxrpc.h

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

128128
#define rxrpc_skb_traces \
129-
EM(rxrpc_skb_eaten_by_unshare, "ETN unshare ") \
130-
EM(rxrpc_skb_eaten_by_unshare_nomem, "ETN unshar-nm") \
131129
EM(rxrpc_skb_get_conn_secured, "GET conn-secd") \
132130
EM(rxrpc_skb_get_conn_work, "GET conn-work") \
133131
EM(rxrpc_skb_get_last_nack, "GET last-nack") \
@@ -139,6 +137,7 @@
139137
EM(rxrpc_skb_new_error_report, "NEW error-rpt") \
140138
EM(rxrpc_skb_new_jumbo_subpacket, "NEW jumbo-sub") \
141139
EM(rxrpc_skb_new_unshared, "NEW unshared ") \
140+
EM(rxrpc_skb_put_call_rx, "PUT call-rx ") \
142141
EM(rxrpc_skb_put_conn_secured, "PUT conn-secd") \
143142
EM(rxrpc_skb_put_conn_work, "PUT conn-work") \
144143
EM(rxrpc_skb_put_error_report, "PUT error-rep") \
@@ -152,6 +151,7 @@
152151
EM(rxrpc_skb_see_recvmsg, "SEE recvmsg ") \
153152
EM(rxrpc_skb_see_reject, "SEE reject ") \
154153
EM(rxrpc_skb_see_rotate, "SEE rotate ") \
154+
EM(rxrpc_skb_see_unshare_nomem, "SEE unshar-nm") \
155155
E_(rxrpc_skb_see_version, "SEE version ")
156156

157157
#define rxrpc_local_traces \

net/rxrpc/ar-internal.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,6 @@ int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
12531253
void rxrpc_kernel_data_consumed(struct rxrpc_call *, struct sk_buff *);
12541254
void rxrpc_new_skb(struct sk_buff *, enum rxrpc_skb_trace);
12551255
void rxrpc_see_skb(struct sk_buff *, enum rxrpc_skb_trace);
1256-
void rxrpc_eaten_skb(struct sk_buff *, enum rxrpc_skb_trace);
12571256
void rxrpc_get_skb(struct sk_buff *, enum rxrpc_skb_trace);
12581257
void rxrpc_free_skb(struct sk_buff *, enum rxrpc_skb_trace);
12591258
void rxrpc_purge_queue(struct sk_buff_head *);

net/rxrpc/call_event.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,8 +342,27 @@ bool rxrpc_input_call_event(struct rxrpc_call *call, struct sk_buff *skb)
342342
if (skb && skb->mark == RXRPC_SKB_MARK_ERROR)
343343
goto out;
344344

345-
if (skb)
346-
rxrpc_input_call_packet(call, skb);
345+
if (skb) {
346+
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
347+
348+
if (sp->hdr.securityIndex != 0 &&
349+
skb_cloned(skb)) {
350+
/* Unshare the packet so that it can be
351+
* modified by in-place decryption.
352+
*/
353+
struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
354+
355+
if (nskb) {
356+
rxrpc_new_skb(nskb, rxrpc_skb_new_unshared);
357+
rxrpc_input_call_packet(call, nskb);
358+
rxrpc_free_skb(nskb, rxrpc_skb_put_call_rx);
359+
} else {
360+
rxrpc_see_skb(skb, rxrpc_skb_see_unshare_nomem);
361+
}
362+
} else {
363+
rxrpc_input_call_packet(call, skb);
364+
}
365+
}
347366

348367
/* If we see our async-event poke, check for timeout trippage. */
349368
now = ktime_get_real();

net/rxrpc/io_thread.c

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,12 @@ static bool rxrpc_extract_abort(struct sk_buff *skb)
168168
/*
169169
* Process packets received on the local endpoint
170170
*/
171-
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
171+
static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff *skb)
172172
{
173173
struct rxrpc_connection *conn;
174174
struct sockaddr_rxrpc peer_srx;
175175
struct rxrpc_skb_priv *sp;
176176
struct rxrpc_peer *peer = NULL;
177-
struct sk_buff *skb = *_skb;
178177
bool ret = false;
179178

180179
skb_pull(skb, sizeof(struct udphdr));
@@ -220,25 +219,6 @@ static bool rxrpc_input_packet(struct rxrpc_local *local, struct sk_buff **_skb)
220219
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_call);
221220
if (sp->hdr.seq == 0)
222221
return rxrpc_bad_message(skb, rxrpc_badmsg_zero_seq);
223-
224-
/* Unshare the packet so that it can be modified for in-place
225-
* decryption.
226-
*/
227-
if (sp->hdr.securityIndex != 0) {
228-
skb = skb_unshare(skb, GFP_ATOMIC);
229-
if (!skb) {
230-
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare_nomem);
231-
*_skb = NULL;
232-
return just_discard;
233-
}
234-
235-
if (skb != *_skb) {
236-
rxrpc_eaten_skb(*_skb, rxrpc_skb_eaten_by_unshare);
237-
*_skb = skb;
238-
rxrpc_new_skb(skb, rxrpc_skb_new_unshared);
239-
sp = rxrpc_skb(skb);
240-
}
241-
}
242222
break;
243223

244224
case RXRPC_PACKET_TYPE_CHALLENGE:
@@ -476,7 +456,7 @@ int rxrpc_io_thread(void *data)
476456
switch (skb->mark) {
477457
case RXRPC_SKB_MARK_PACKET:
478458
skb->priority = 0;
479-
if (!rxrpc_input_packet(local, &skb))
459+
if (!rxrpc_input_packet(local, skb))
480460
rxrpc_reject_packet(local, skb);
481461
trace_rxrpc_rx_done(skb->mark, skb->priority);
482462
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)