Re: [PATCH net v3 04/14] rxrpc: Fix packet encryption error handling
From: Simon Horman <horms@kernel.org>
Date: 2026-07-20 14:52:39
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. https://sashiko.dev/#/patchset/20260713081022.2186481-8-dhowells@redhat.com?part=4 ---
--- Patch [4]: [PATCH net v3 04/14] rxrpc: Fix packet encryption error handling ---commit 01fc73762a5a2162d360f4b9e10f752440ebbd67 Author: David Howells [off-list ref] rxrpc: Fix packet encryption error handling This commit modifies the error handling in rxrpc_send_data() when secure_packet() fails. It treats -ENOMEM as a retryable error and marks other errors as terminal by setting the RXRPC_CALL_TX_ERROR flag to abort the call. Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
quoted hunk ↗ jump to hunk
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c --- a/net/rxrpc/sendmsg.c +++ b/net/rxrpc/sendmsg.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -440,12 +446,21 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, /* add the packet to the send queue if it's now full */ if (!txb->space || (msg_data_left(msg) == 0 && !more)) { - if (msg_data_left(msg) == 0 && !more) - txb->flags |= RXRPC_LAST_PACKET; - + /* Do any required crypto. If this fails, it could + * have corrupted the txbuf content with a partial + * encrypt. Assume that ENOMEM is retryable, but + * everything else is terminal. + */ ret = call->security->secure_packet(call, txb); - if (ret < 0) + if (ret < 0) { + if (ret == -ENOMEM) + goto maybe_error;
[Severity: High]
If secure_packet() fails with -ENOMEM, does this jump to maybe_error bypass
rxrpc_queue_packet() but still return a successful write length?
Looking at rxrpc_send_data(), the user data has already been copied into txb
and 'copied' has been incremented. The maybe_error path handles this:
maybe_error:
if (copied) {
...
goto success;
}
Because copied > 0, it jumps to success and returns the number of bytes
copied, including the data in the unqueued txb.
Could this mislead userspace into thinking the data was sent when it was
actually dropped, leaving the unencrypted packet permanently stuck in
call->tx_pending and leading to an application hang?
+ set_bit(RXRPC_CALL_TX_ERROR, &call->flags); goto out; + } + + if (msg_data_left(msg) == 0 && !more) + txb->flags |= RXRPC_LAST_PACKET; rxrpc_queue_packet(rx, call, txb, notify_end_tx); txb = NULL; }