Commit 122d74fa authored by David Howells's avatar David Howells Committed by David S. Miller

rxrpc: Fix use-after-free in rxrpc_receive_data()

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: e2de6c40 ("rxrpc: Use info in skbuff instead of reparsing a jumbo packet")
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 55cd9f67
...@@ -413,7 +413,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb) ...@@ -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); struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
enum rxrpc_call_state state; enum rxrpc_call_state state;
unsigned int j; unsigned int j, nr_subpackets;
rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0; rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0;
rxrpc_seq_t seq0 = sp->hdr.seq, hard_ack; rxrpc_seq_t seq0 = sp->hdr.seq, hard_ack;
bool immediate_ack = false, jumbo_bad = false; bool immediate_ack = false, jumbo_bad = false;
...@@ -457,7 +457,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb) ...@@ -457,7 +457,8 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
call->ackr_prev_seq = seq0; call->ackr_prev_seq = seq0;
hard_ack = READ_ONCE(call->rx_hard_ack); 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) { if (call->nr_jumbo_bad > 3) {
ack = RXRPC_ACK_NOSPACE; ack = RXRPC_ACK_NOSPACE;
ack_serial = serial; ack_serial = serial;
...@@ -465,11 +466,11 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb) ...@@ -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_serial_t serial = sp->hdr.serial + j;
rxrpc_seq_t seq = seq0 + j; rxrpc_seq_t seq = seq0 + j;
unsigned int ix = seq & RXRPC_RXTX_BUFF_MASK; 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); bool last = terminal && (sp->rx_flags & RXRPC_SKB_INCL_LAST);
u8 flags, annotation = j; u8 flags, annotation = j;
...@@ -506,7 +507,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb) ...@@ -506,7 +507,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
} }
if (call->rxtx_buffer[ix]) { 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); &jumbo_bad);
if (ack != RXRPC_ACK_DUPLICATE) { if (ack != RXRPC_ACK_DUPLICATE) {
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) ...@@ -564,6 +565,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb)
* ring. * ring.
*/ */
skb = NULL; skb = NULL;
sp = NULL;
} }
if (last) { if (last) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment