Re: [PATCH net v9 05/14] rxrpc: Fix sendmsg length
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-08 20:49:43
Also in:
lkml, stable
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 0 · Low: 1 - [Low] "rxrpc: Fix sendmsg length" carries a `Fixes:` tag and `cc: stable@vger.kernel.org` while its own commit message states… -- rxrpc: Fix sendmsg length This makes rxrpc_send_data() consistently use its len argument rather than msg->msg_iter.count, decrementing len as data is copied, and limits each copy to the smaller of len and msg_data_left(msg).
quoted hunk ↗ jump to hunk
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c index 1d66e98081624..565799548102c 100644 --- a/net/rxrpc/sendmsg.c +++ b/net/rxrpc/sendmsg.c@@ -379,9 +379,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, ret = -EMSGSIZE; if (call->tx_total_len != -1) { - if (len - copied > call->tx_total_len) + if (len > call->tx_total_len) goto maybe_error; - if (!more && len - copied != call->tx_total_len) + if (!more && len != call->tx_total_len) goto maybe_error; }
[Severity: Low]
This isn't a bug in the code, but a question about the tagging. The
commit carries:
Fixes: 382d7974de31 ("RxRPC: Use iov_iter_count() in rxrpc_send_data() instead of the len argument")
cc: stable@vger.kernel.org
while the message itself says:
Note that, currently, all the callers have len and msg->msg_iter.count the
same and so the problem won't occur.
Walking the callers agrees with that statement. rxrpc_do_sendmsg()
forwards the sendmsg() len that the syscall also used to build msg_iter.
In fs/afs/rxrpc.c, afs_make_call() and afs_send_simple_reply() pass
msg_data_left(&msg):
ret = rxrpc_kernel_send_data(call->net->socket, rxcall, &msg,
msg_data_left(&msg),
afs_notify_end_request_tx);
and afs_send_empty_reply() passes 0 for a zero-length iterator. In
net/rxrpc/rxperf.c, rxperf_process_call() passes the same local len used
to build the iterator:
iov_iter_bvec(&msg.msg_iter, WRITE, &bv, 1, len);
msg.msg_flags = MSG_MORE;
n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
len, rxperf_notify_end_reply_tx);
So no in-tree caller can diverge. Given there is no symptom, reproducer
or affected caller named, are the Fixes: and cc: stable tags intended
here?
What stable trees would pick up instead is a rewrite of the sendmsg data
loop: len becomes mutable and is decremented in the loop, the copy bound
becomes min3(), the loop termination test gains a second clause, and the
-EMSGSIZE re-check that is re-executed via the reload label now tests the
decremented len instead of len - copied.
quoted hunk ↗ jump to hunk
@@ -405,7 +405,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, * the security header is going to be in the padded * region (enc blocksize), but the trailer is not. */ - remain = more ? INT_MAX : msg_data_left(msg); + remain = more ? INT_MAX : len; txb = call->conn->security->alloc_txbuf(call, remain, sk->sk_allocation); if (!txb) { ret = -ENOMEM;@@ -416,8 +416,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, _debug("append"); /* append next segment of data to the current buffer */ - if (msg_data_left(msg) > 0) { - size_t copy = umin(txb->space, msg_data_left(msg)); + if (len > 0) { + size_t copy = min3(txb->space, len, msg_data_left(msg)); _debug("add %zu", copy); if (!copy_from_iter_full(txb->data + txb->offset,@@ -428,6 +428,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, txb->len += copy; txb->offset += copy; copied += copy; + len -= copy; if (call->tx_total_len != -1) call->tx_total_len -= copy; }@@ -439,8 +440,8 @@ 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) + (len == 0 && !more)) { + if (len == 0 && !more) txb->flags |= RXRPC_LAST_PACKET; ret = call->security->secure_packet(call, txb);@@ -449,7 +450,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, rxrpc_queue_packet(rx, call, txb, notify_end_tx); txb = NULL; } - } while (msg_data_left(msg) > 0); + } while (len > 0 && msg_data_left(msg) > 0);
Making len mutable also appears to be a prerequisite for the later patch
in the series, "rxrpc: Fix packet encryption error handling", whose rewind
path tests:
if (copied && !more && !len) {
which only behaves as intended once len is decremented here. Could that
dependency be mentioned in the commit message?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907113743.1453210-1-dhowells%40redhat.com