* [PATCH net] rxrpc: Fix use-after-free in rxrpc_receive_data()
@ 2020-01-24 23:08 David Howells
2020-01-27 9:57 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: David Howells @ 2020-01-24 23:08 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
The subpacket scanning loop in rxrpc_receive_data() references the
subpacket count in the private data part of the sk_buff in the loop
termination condition. However, when the final subpacket is pasted into
the ring buffer, the function is no longer has a ref on the sk_buff and
should not be looking at sp->* any more. This point is actually marked in
the code when skb is cleared (but sp is not - which is an error).
Fix this by caching sp->nr_subpackets in a local variable and using that
instead.
Also clear 'sp' to catch accesses after that point.
This can show up as an oops in rxrpc_get_skb() if sp->nr_subpackets gets
trashed by the sk_buff getting freed and reused in the meantime.
Fixes: e2de6c404898 ("rxrpc: Use info in skbuff instead of reparsing a jumbo packet")
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/input.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 86bd133b4fa0..96d54e5bf7bc 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -413,7 +413,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
enum rxrpc_call_state state;
- unsigned int j;
+ unsigned int j, nr_subpackets;
rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0;
rxrpc_seq_t seq0 = sp->hdr.seq, hard_ack;
bool immediate_ack = false, jumbo_bad = false;
@@ -457,7 +457,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
call->ackr_prev_seq = seq0;
hard_ack = READ_ONCE(call->rx_hard_ack);
- if (sp->nr_subpackets > 1) {
+ nr_subpackets = sp->nr_subpackets;
+ if (nr_subpackets > 1) {
if (call->nr_jumbo_bad > 3) {
ack = RXRPC_ACK_NOSPACE;
ack_serial = serial;
@@ -465,11 +466,11 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
}
}
- for (j = 0; j < sp->nr_subpackets; j++) {
+ for (j = 0; j < nr_subpackets; j++) {
rxrpc_serial_t serial = sp->hdr.serial + j;
rxrpc_seq_t seq = seq0 + j;
unsigned int ix = seq & RXRPC_RXTX_BUFF_MASK;
- bool terminal = (j == sp->nr_subpackets - 1);
+ bool terminal = (j == nr_subpackets - 1);
bool last = terminal && (sp->rx_flags & RXRPC_SKB_INCL_LAST);
u8 flags, annotation = j;
@@ -506,7 +507,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
}
if (call->rxtx_buffer[ix]) {
- rxrpc_input_dup_data(call, seq, sp->nr_subpackets > 1,
+ rxrpc_input_dup_data(call, seq, nr_subpackets > 1,
&jumbo_bad);
if (ack != RXRPC_ACK_DUPLICATE) {
ack = RXRPC_ACK_DUPLICATE;
@@ -564,6 +565,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
* ring.
*/
skb = NULL;
+ sp = NULL;
}
if (last) {
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net] rxrpc: Fix use-after-free in rxrpc_receive_data()
2020-01-24 23:08 [PATCH net] rxrpc: Fix use-after-free in rxrpc_receive_data() David Howells
@ 2020-01-27 9:57 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2020-01-27 9:57 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
From: David Howells <dhowells@redhat.com>
Date: Fri, 24 Jan 2020 23:08:04 +0000
> The subpacket scanning loop in rxrpc_receive_data() references the
> subpacket count in the private data part of the sk_buff in the loop
> termination condition. However, when the final subpacket is pasted into
> the ring buffer, the function is no longer has a ref on the sk_buff and
> should not be looking at sp->* any more. This point is actually marked in
> the code when skb is cleared (but sp is not - which is an error).
>
> Fix this by caching sp->nr_subpackets in a local variable and using that
> instead.
>
> Also clear 'sp' to catch accesses after that point.
>
> This can show up as an oops in rxrpc_get_skb() if sp->nr_subpackets gets
> trashed by the sk_buff getting freed and reused in the meantime.
>
> Fixes: e2de6c404898 ("rxrpc: Use info in skbuff instead of reparsing a jumbo packet")
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-01-27 9:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 23:08 [PATCH net] rxrpc: Fix use-after-free in rxrpc_receive_data() David Howells
2020-01-27 9:57 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.