Skip to content

Commit 2ad6691

Browse files
dhowellsdavem330
authored andcommitted
rxrpc: Fix race between incoming ACK parser and retransmitter
There's a race between the retransmission code and the received ACK parser. The problem is that the retransmission loop has to drop the lock under which it is iterating through the transmission buffer in order to transmit a packet, but whilst the lock is dropped, the ACK parser can crank the Tx window round and discard the packets from the buffer. The retransmission code then updated the annotations for the wrong packet and a later retransmission thought it had to retransmit a packet that wasn't there, leading to a NULL pointer dereference. Fix this by: (1) Moving the annotation change to before we drop the lock prior to transmission. This means we can't vary the annotation depending on the outcome of the transmission, but that's fine - we'll retransmit again later if it failed now. (2) Skipping the packet if the skb pointer is NULL. The following oops was seen: BUG: kernel NULL pointer dereference, address: 000000000000002d Workqueue: krxrpcd rxrpc_process_call RIP: 0010:rxrpc_get_skb+0x14/0x8a ... Call Trace: rxrpc_resend+0x331/0x41e ? get_vtime_delta+0x13/0x20 rxrpc_process_call+0x3c0/0x4ac process_one_work+0x18f/0x27f worker_thread+0x1a3/0x247 ? create_worker+0x17d/0x17d kthread+0xe6/0xeb ? kthread_delayed_work_timer_fn+0x83/0x83 ret_from_fork+0x1f/0x30 Fixes: 248f219 ("rxrpc: Rewrite the data and ack handling code") Signed-off-by: David Howells <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 9798278 commit 2ad6691

File tree

1 file changed

+11
-18
lines changed

1 file changed

+11
-18
lines changed

net/rxrpc/call_event.c

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,18 @@ static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
248248
if (anno_type != RXRPC_TX_ANNO_RETRANS)
249249
continue;
250250

251+
/* We need to reset the retransmission state, but we need to do
252+
* so before we drop the lock as a new ACK/NAK may come in and
253+
* confuse things
254+
*/
255+
annotation &= ~RXRPC_TX_ANNO_MASK;
256+
annotation |= RXRPC_TX_ANNO_RESENT;
257+
call->rxtx_annotations[ix] = annotation;
258+
251259
skb = call->rxtx_buffer[ix];
260+
if (!skb)
261+
continue;
262+
252263
rxrpc_get_skb(skb, rxrpc_skb_got);
253264
spin_unlock_bh(&call->lock);
254265

@@ -262,24 +273,6 @@ static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
262273

263274
rxrpc_free_skb(skb, rxrpc_skb_freed);
264275
spin_lock_bh(&call->lock);
265-
266-
/* We need to clear the retransmit state, but there are two
267-
* things we need to be aware of: A new ACK/NAK might have been
268-
* received and the packet might have been hard-ACK'd (in which
269-
* case it will no longer be in the buffer).
270-
*/
271-
if (after(seq, call->tx_hard_ack)) {
272-
annotation = call->rxtx_annotations[ix];
273-
anno_type = annotation & RXRPC_TX_ANNO_MASK;
274-
if (anno_type == RXRPC_TX_ANNO_RETRANS ||
275-
anno_type == RXRPC_TX_ANNO_NAK) {
276-
annotation &= ~RXRPC_TX_ANNO_MASK;
277-
annotation |= RXRPC_TX_ANNO_UNACK;
278-
}
279-
annotation |= RXRPC_TX_ANNO_RESENT;
280-
call->rxtx_annotations[ix] = annotation;
281-
}
282-
283276
if (after(call->tx_hard_ack, seq))
284277
seq = call->tx_hard_ack;
285278
}

0 commit comments

Comments
 (0)