Re: [PATCH] rxrpc: Fix read+write past skb_headlen in soft-ACK parser

[David Howells <dhowells@redhat.com>]

Jeffrey E Altman <jaltman@auristor.com> wrote:

> Aborting the call because skb_condense() was unable to consolidate the rx ack
> packet data is an unfriendly thing to do.
> 
> As suggested by the commit message, copying the data before processing would
> be a friendlier solution to the identified problem.

Agreed.  Something along the lines of the attached.  It still needs a bit more
polishing, though.

David
---
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 24aceb183c2c..dde87b058a23 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -963,21 +963,30 @@ static void rxrpc_input_soft_acks(struct rxrpc_call *call,
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
 	struct rxrpc_txqueue *tq = call->tx_queue;
 	unsigned long extracted = ~0UL;
-	unsigned int nr = 0;
+	unsigned int nr = 0, nsack;
 	rxrpc_seq_t seq = call->acks_hard_ack + 1;
 	rxrpc_seq_t lowest_nak = seq + sp->ack.nr_acks;
-	u8 *acks = skb->data + sizeof(struct rxrpc_wire_header) + sizeof(struct rxrpc_ackpacket);
+	u8 sack[256] __aligned(sizeof(unsigned long));
+	u8 *acks = sack;
 
 	_enter("%x,%x,%u", tq->qbase, seq, sp->ack.nr_acks);
 
 	while (after(seq, tq->qbase + RXRPC_NR_TXQUEUE - 1))
 		tq = tq->next;
 
+	/* Extract a SACK table.  A SACK table can hold up to 256*8 ACK bits. */
+	memset(sack, 0, sizeof(sack));
+	nsack = umin(sp->ack.nr_acks, 256);
+	if (skb_copy_bits(skb,
+			  sizeof(struct rxrpc_wire_header) + sizeof(struct rxrpc_ackpacket),
+			  sack, nsack) < 0)
+		return;
+
 	for (unsigned int i = 0; i < sp->ack.nr_acks; i++) {
 		/* Decant ACKs until we hit a txqueue boundary. */
 		shiftr_adv_rotr(acks, extracted);
 		if (i == 256) {
-			acks -= i;
+			acks = sack;
 			i = 0;
 		}
 		seq++;