rxrpc: Also unshare DATA/RESPONSE packets when paged frags are present

From: 'Simon Horman' <[email protected]>

This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev
---
rxrpc: Also unshare DATA/RESPONSE packets when paged frags are present

This commit updates the DATA and RESPONSE packet handlers in rxrpc to copy the skb into a linear one if there are paged fragments present. This ensures the security handler operates on a fully linear skb during decryption.

net/rxrpc/call_event.c bool rxrpc_input_call_event(struct rxrpc_call *call) · line 340

Quoted 13 lines

net/rxrpc/call_event.c +1 -1

index fdd683261226..6c924ef55208 100644
@@ -334,7 +334,7 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
 
 			if (sp->hdr.type == RXRPC_PACKET_TYPE_DATA &&
 			    sp->hdr.securityIndex != 0 &&
-			    skb_cloned(skb)) {
+			    (skb_cloned(skb) || skb->data_len)) {
 				/* Unshare the packet so that it can be
 				 * modified by in-place decryption.
 				 */

Since modern NIC drivers commonly use page pools where packet payloads are placed in page fragments, skb->data_len will be greater than zero for almost all standard incoming packets.

By taking this path, the code calls skb_copy() right after this check. Will this force a full packet copy for nearly all incoming RxRPC DATA packets, bypassing zero-copy receive paths and causing a large performance regression?

Additionally, skb_copy() fully linearizes the packet by allocating a single contiguous buffer for the entire packet size. For RxRPC Jumbo packets or UDP GRO-aggregated packets (which can reach up to ~64KB), this requires an order-4 page allocation.

Could relying on an order-4 GFP_ATOMIC allocation cause reliable OOM failures and packet drops on fragmented systems?

If the goal is to ensure the skb is writable for in-place decryption, would it be safer to use skb_cow_data() instead of forcing a full contiguous linearization?

Simon says:

I note that this patch included

Fixes d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")

And that the suggestion in the last paragraph above would reintroduce back to the problem addressed by d0d5c0cd1e71.

The DATA-packet handler in rxrpc_input_call_event() and the RESPONSE handler in rxrpc_verify_response() copy the skb to a linear one before calling into the security ops only when skb_cloned() is true. An skb that is not cloned but still carries paged fragments (skb->data_len != 0) falls through to the in-place decryption path, which binds the frag pages directly into the AEAD/skcipher SGL via skb_to_sgvec().

Extend the gate so that any skb with non-linear data is also copied, ensuring the security handler always operates on a fully linear skb. The OOM/trace handling already in place is reused.

Fixes d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")

Signed-off-by Hyunwoo Kim <[email protected]>

---
 net/rxrpc/call_event.c | 2 +-
 net/rxrpc/conn_event.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

net/rxrpc/call_event.c +0 -0

index fdd683261226..6c924ef55208 100644

Quoted 24 lines · Hyunwoo Kim

The DATA-packet handler in rxrpc_input_call_event() and the RESPONSE handler in rxrpc_verify_response() copy the skb to a linear one before calling into the security ops only when skb_cloned() is true. An skb that is not cloned but still carries paged fragments (skb->data_len != 0) falls through to the in-place decryption path, which binds the frag pages directly into the AEAD/skcipher SGL via skb_to_sgvec().

Extend the gate so that any skb with non-linear data is also copied, ensuring the security handler always operates on a fully linear skb. The OOM/trace handling already in place is reused.

Fixes d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")

Signed-off-by Hyunwoo Kim <[email protected]>

---
 net/rxrpc/call_event.c | 2 +-
 net/rxrpc/conn_event.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

net/rxrpc/call_event.c +0 -0

index fdd683261226..6c924ef55208 100644

net/rxrpc/call_event.c bool rxrpc_input_call_event(struct rxrpc_call *call) · line 337

Quoted 5 lines

net/rxrpc/call_event.c +1 -1

@@ -334,7 +334,7 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
 
 			if (sp->hdr.type == RXRPC_PACKET_TYPE_DATA &&
 			    sp->hdr.securityIndex != 0 &&
-			    skb_cloned(skb)) {
+			    (skb_cloned(skb) || skb->data_len)) {

It's recommended to use skb_is_nonlinear() instead of open-coding skb->data_len.

Quoted 11 lines

net/rxrpc/call_event.c +0 -0

 				/* Unshare the packet so that it can be
 				 * modified by in-place decryption.
 				 */

net/rxrpc/conn_event.c +0 -0

index a2130d25aaa9..eab7c5f2517a 100644
@@ -245,7 +245,7 @@ static int rxrpc_verify_response(struct rxrpc_connection *conn,
 {
 	int ret;
 

net/rxrpc/conn_event.c static int rxrpc_verify_response(struct rxrpc_connection *conn, · line 248

Quoted 2 lines

net/rxrpc/conn_event.c +1 -1

-	if (skb_cloned(skb)) {
+	if (skb_cloned(skb) || skb->data_len) {

Ditto.

net/rxrpc/conn_event.c static int rxrpc_verify_response(struct rxrpc_connection *conn, · line 251

Quoted 4 lines

net/rxrpc/conn_event.c +0 -0

 		/* Copy the packet if shared so that we can do in-place
 		 * decryption.
 		 */

Regards,
Qingfang

Quoted 4 lines

net/rxrpc/conn_event.c static int rxrpc_verify_response(struct rxrpc_connection *conn, · line 248

Quoted 2 lines

net/rxrpc/conn_event.c +1 -1

-	if (skb_cloned(skb)) {
+	if (skb_cloned(skb) || skb->data_len) {  

Ditto.

Thank you for the review.

Quoted 4 lines · On Fri, 8 May 2026 15:07:49 +0900 Hyunwoo Kim wrote:

Quoted 3 lines

net/rxrpc/conn_event.c static int rxrpc_verify_response(struct rxrpc_connection *conn, · line 248

Quoted 2 lines

net/rxrpc/conn_event.c +1 -1

-	if (skb_cloned(skb)) {
+	if (skb_cloned(skb) || skb->data_len) {

Ditto.

Thank you for the review.

Maybe skb_ensure_writable() ?