* [PATCH net v11 01/17] rxrpc: Revert "rxrpc: rxperf: Fix missing decoding of terminal magic cookie"
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 02/17] rxrpc: Fix rxperf test rxgk key kvno to be 0 David Howells
` (16 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel
Revert commit c34d999ca3145d9fe858258cc3342ec493f47d2e:
The rxperf RPCs seem to have a magic cookie at the end of the request
that was failing to be taken account of by the unmarshalling of the
request. Fix the rxperf code to expect this.
Actually, this isn't true; it's just that other Rx implementations ignore
the extra data in the request and so my test programs are sending too much
data without noticeable consequence.
Fixes: c34d999ca314 ("rxrpc: rxperf: Fix missing decoding of terminal magic cookie")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
net/rxrpc/rxperf.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index b8df6d22314d..f1f41151589c 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -483,18 +483,6 @@ static int rxperf_deliver_request(struct rxperf_call *call)
call->unmarshal++;
fallthrough;
case 2:
- ret = rxperf_extract_data(call, true);
- if (ret < 0)
- return ret;
-
- /* Deal with the terminal magic cookie. */
- call->iov_len = 4;
- call->kvec[0].iov_len = call->iov_len;
- call->kvec[0].iov_base = call->tmp;
- iov_iter_kvec(&call->iter, READ, call->kvec, 1, call->iov_len);
- call->unmarshal++;
- fallthrough;
- case 3:
ret = rxperf_extract_data(call, false);
if (ret < 0)
return ret;
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 02/17] rxrpc: Fix rxperf test rxgk key kvno to be 0
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
2026-09-23 13:36 ` [PATCH net v11 01/17] rxrpc: Revert "rxrpc: rxperf: Fix missing decoding of terminal magic cookie" David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 03/17] rxrpc: Fix update of call->tx_pending without holding lock David Howells
` (15 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman
Fix the test rxgk keys in the rxperf test server to have kvno 0 to match
other implementations.
Fixes: aa2199088a39 ("rxrpc: rxperf: Add test RxGK server keys")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
net/rxrpc/rxperf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index f1f41151589c..26f97e1dd388 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -586,7 +586,7 @@ static int rxperf_add_yfs_rxgk_key(struct key *keyring, u32 enctype)
for (int i = 0; i < krb5->key_len; i++)
key[i] = i;
- sprintf(name, "%u:6:1:%u", RX_PERF_SERVICE, enctype);
+ sprintf(name, "%u:6:0:%u", RX_PERF_SERVICE, enctype);
kref = key_create_or_update(make_key_ref(keyring, true),
"rxrpc_s", name,
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 03/17] rxrpc: Fix update of call->tx_pending without holding lock
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
2026-09-23 13:36 ` [PATCH net v11 01/17] rxrpc: Revert "rxrpc: rxperf: Fix missing decoding of terminal magic cookie" David Howells
2026-09-23 13:36 ` [PATCH net v11 02/17] rxrpc: Fix rxperf test rxgk key kvno to be 0 David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 04/17] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data() David Howells
` (14 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
Currently, rxrpc_send_data() updates call->tx_pending just before it
returns - but it won't be holding the call->user_mutex when it does this if
a wait was interrupted by a signal. This would allow a parallel sendmsg()
to race.
Further, both the callers of rxrpc_send_data() call it with the lock held,
and then it returns an indication through the parameter list to say whether
it has dropped the lock or not - after which the callers both just drop the
lock if it's still held.
Fix this by:
(1) Moving the release of call->user_mutex down into rxrpc_send_data() and
get rid of the indicator parameter. This makes it easier to see where
the lock is held.
(2) After waiting, if the attempt to reacquire the mutex is interrupted,
just return directly there rather than going to out_unlock
(3) Restricting the txb variable to inside the buffering loop and leaving
->tx_pending set until we've queued the buffer.
Note that there's a slight change in behaviour in that wait_for_space
failure now doesn't check for completion because it doesn't hold the call
user_mutex. The caller, however, should re-issue the send and pick up any
error at a second attempt.
Fixes: b0f571ecd794 ("rxrpc: Fix locking in rxrpc's sendmsg")
Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
net/rxrpc/sendmsg.c | 63 +++++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 34 deletions(-)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index ed2c9a51005a..fb8d48418882 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -320,10 +320,9 @@ static int rxrpc_alloc_txqueue(struct sock *sk, struct rxrpc_call *call)
static int rxrpc_send_data(struct rxrpc_sock *rx,
struct rxrpc_call *call,
struct msghdr *msg, size_t len,
- rxrpc_notify_end_tx_t notify_end_tx,
- bool *_dropped_lock)
+ rxrpc_notify_end_tx_t notify_end_tx)
+ __releases(&call->user_mutex)
{
- struct rxrpc_txbuf *txb;
struct sock *sk = &rx->sk;
enum rxrpc_call_state state;
long timeo;
@@ -334,30 +333,26 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
call->cid, call->call_id, call->rx_consumed,
0, -EPROTO);
- return -EPROTO;
+ ret = -EPROTO;
+ goto out_unlock;
}
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
ret = rxrpc_wait_to_be_connected(call, &timeo);
if (ret < 0)
- return ret;
+ goto out_unlock;
if (call->conn->state == RXRPC_CONN_CLIENT_UNSECURED) {
ret = rxrpc_init_client_conn_security(call->conn);
if (ret < 0)
- return ret;
+ goto out_unlock;
}
/* this should be in poll */
sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
reload:
- txb = call->tx_pending;
- call->tx_pending = NULL;
- if (txb)
- rxrpc_see_txbuf(txb, rxrpc_txbuf_see_send_more);
-
ret = -EPIPE;
if (sk->sk_shutdown & SEND_SHUTDOWN)
goto maybe_error;
@@ -386,6 +381,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
}
do {
+ struct rxrpc_txbuf *txb = call->tx_pending;
+
if (!txb) {
size_t remain;
@@ -411,6 +408,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
ret = -ENOMEM;
goto maybe_error;
}
+ call->tx_pending = txb;
+ } else {
+ rxrpc_see_txbuf(txb, rxrpc_txbuf_see_send_more);
}
_debug("append");
@@ -445,9 +445,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
ret = call->security->secure_packet(call, txb);
if (ret < 0)
- goto out;
+ goto out_unlock;
rxrpc_queue_packet(rx, call, txb, notify_end_tx);
- txb = NULL;
+ call->tx_pending = NULL;
}
} while (msg_data_left(msg) > 0);
@@ -456,45 +456,46 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
if (rxrpc_call_is_complete(call) &&
call->error < 0)
ret = call->error;
-out:
- call->tx_pending = txb;
+out_unlock:
+ mutex_unlock(&call->user_mutex);
_leave(" = %d", ret);
return ret;
call_terminated:
- rxrpc_put_txbuf(txb, rxrpc_txbuf_put_send_aborted);
- _leave(" = %d", call->error);
- return call->error;
+ ret = call->error;
+ goto out_unlock;
maybe_error:
if (copied)
goto success;
- goto out;
+ goto out_unlock;
efault:
ret = -EFAULT;
- goto out;
+ goto out_unlock;
wait_for_space:
ret = -EAGAIN;
if (msg->msg_flags & MSG_DONTWAIT)
goto maybe_error;
mutex_unlock(&call->user_mutex);
- *_dropped_lock = true;
+
ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
msg->msg_flags & MSG_WAITALL);
if (ret < 0)
- goto maybe_error;
+ goto out_nolock;
if (call->interruptibility == RXRPC_INTERRUPTIBLE) {
if (mutex_lock_interruptible(&call->user_mutex) < 0) {
ret = sock_intr_errno(timeo);
- goto maybe_error;
+ goto out_nolock;
}
} else {
mutex_lock(&call->user_mutex);
}
- *_dropped_lock = false;
goto reload;
+out_nolock:
+ _leave(" = %d [intr]", ret);
+ return copied ?: ret;
}
/*
@@ -660,7 +661,6 @@ rxrpc_new_client_call_for_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg,
int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
{
struct rxrpc_call *call;
- bool dropped_lock = false;
int ret;
struct rxrpc_send_params p = {
@@ -769,16 +769,15 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
ret = 0;
break;
case RXRPC_CMD_SEND_DATA:
- ret = rxrpc_send_data(rx, call, msg, len, NULL, &dropped_lock);
- break;
+ ret = rxrpc_send_data(rx, call, msg, len, NULL);
+ goto error_put;
default:
ret = -EINVAL;
break;
}
out_put_unlock:
- if (!dropped_lock)
- mutex_unlock(&call->user_mutex);
+ mutex_unlock(&call->user_mutex);
error_put:
rxrpc_put_call(call, rxrpc_call_put_sendmsg);
_leave(" = %d", ret);
@@ -808,7 +807,6 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
struct msghdr *msg, size_t len,
rxrpc_notify_end_tx_t notify_end_tx)
{
- bool dropped_lock = false;
int ret;
_enter("{%d},", call->debug_id);
@@ -819,12 +817,9 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
mutex_lock(&call->user_mutex);
ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg, len,
- notify_end_tx, &dropped_lock);
+ notify_end_tx);
if (ret == -ESHUTDOWN)
ret = call->error;
-
- if (!dropped_lock)
- mutex_unlock(&call->user_mutex);
_leave(" = %d", ret);
return ret;
}
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 04/17] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (2 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 03/17] rxrpc: Fix update of call->tx_pending without holding lock David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:36 ` [PATCH net v11 05/17] afs: Fix afs to abort the rxrpc call on send error David Howells
` (13 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
David Laight, stable
Fix rxrpc_kernel_send_data() to loop around if it detects a short send.
David Laight suggested doing it here rather than wrapping all the calls in
loops. Further, remove the len argument and use the iterator count instead
and return 0 on success, not the amount copied.
Note this is also a prerequisite for changing the way rxrpc_send_data()
works to return a short send rather than an error if some data was
buffered.
Fixes: 651350d10f93 ("[AF_RXRPC]: Add an interface to the AF_RXRPC module for the AFS filesystem to use")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
Documentation/networking/rxrpc.rst | 6 ++++--
fs/afs/rxrpc.c | 32 ++++++++++++------------------
include/net/af_rxrpc.h | 5 ++---
net/rxrpc/rxperf.c | 25 ++++++++++-------------
net/rxrpc/sendmsg.c | 27 +++++++++++++++++--------
5 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 8926dab8e2e6..01ad12fdf305 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -870,7 +870,6 @@ The kernel interface functions are as follows:
int rxrpc_kernel_send_data(struct socket *sock,
struct rxrpc_call *call,
struct msghdr *msg,
- size_t len,
rxrpc_notify_end_tx_t notify_end_rx);
This is used to supply either the request part of a client call or the
@@ -880,13 +879,16 @@ The kernel interface functions are as follows:
MSG_MORE if there will be subsequent data sends for this call.
The msg must not specify a destination address, control data or any flags
- other than MSG_MORE. len is the total amount of data to transmit.
+ other than MSG_MORE.
notify_end_rx can be NULL or it can be used to specify a function to be
called when the call changes state to end the Tx phase. This function is
called with a spinlock held to prevent the last DATA packet from being
transmitted until the function returns.
+ It returns 0 if all the data is queued and a negative error code on
+ failure.
+
(#) Receive data from a call::
int rxrpc_kernel_recv_data(struct socket *sock,
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index d82916657a3d..bcb8bb15d471 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -412,8 +412,7 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
msg.msg_controllen = 0;
msg.msg_flags = MSG_WAITALL | (call->write_iter ? MSG_MORE : 0);
- ret = rxrpc_kernel_send_data(call->net->socket, rxcall,
- &msg, call->request_size,
+ ret = rxrpc_kernel_send_data(call->net->socket, rxcall, &msg,
afs_notify_end_request_tx);
if (ret < 0)
goto error_do_abort;
@@ -425,7 +424,6 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
ret = rxrpc_kernel_send_data(call->net->socket,
call->rxcall, &msg,
- iov_iter_count(&msg.msg_iter),
afs_notify_end_request_tx);
*call->write_iter = msg.msg_iter;
@@ -871,7 +869,7 @@ void afs_send_empty_reply(struct afs_call *call)
msg.msg_controllen = 0;
msg.msg_flags = 0;
- switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg, 0,
+ switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
afs_notify_end_reply_tx)) {
case 0:
_leave(" [replied]");
@@ -897,7 +895,7 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
struct afs_net *net = call->net;
struct msghdr msg;
struct kvec iov[1];
- int n;
+ int ret;
_enter("");
@@ -912,21 +910,17 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
msg.msg_controllen = 0;
msg.msg_flags = 0;
- n = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg, len,
- afs_notify_end_reply_tx);
- if (n >= 0) {
- /* Success */
- _leave(" [replied]");
- return;
- }
-
- if (n == -ENOMEM) {
- _debug("oom");
- rxrpc_kernel_abort_call(net->socket, call->rxcall,
- RXGEN_SS_MARSHAL, -ENOMEM,
- afs_abort_oom);
+ ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
+ afs_notify_end_reply_tx);
+ if (ret < 0) {
+ if (ret == -ENOMEM) {
+ _debug("oom");
+ rxrpc_kernel_abort_call(net->socket, call->rxcall,
+ RXGEN_SS_MARSHAL, -ENOMEM,
+ afs_abort_oom);
+ }
+ _leave(" [error]");
}
- _leave(" [error]");
}
/*
diff --git a/include/net/af_rxrpc.h b/include/net/af_rxrpc.h
index 0fb4c41c9bbf..f3980348ed34 100644
--- a/include/net/af_rxrpc.h
+++ b/include/net/af_rxrpc.h
@@ -64,9 +64,8 @@ struct rxrpc_call *rxrpc_kernel_begin_call(struct socket *sock,
bool upgrade,
enum rxrpc_interruptibility interruptibility,
unsigned int debug_id);
-int rxrpc_kernel_send_data(struct socket *, struct rxrpc_call *,
- struct msghdr *, size_t,
- rxrpc_notify_end_tx_t);
+int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
+ struct msghdr *msg, rxrpc_notify_end_tx_t notify_end_tx);
int rxrpc_kernel_recv_data(struct socket *, struct rxrpc_call *,
struct iov_iter *, size_t *, bool, u32 *, u16 *);
bool rxrpc_kernel_abort_call(struct socket *, struct rxrpc_call *,
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 26f97e1dd388..981c0596c774 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -501,8 +501,8 @@ static int rxperf_process_call(struct rxperf_call *call)
struct msghdr msg = {};
struct bio_vec bv;
struct kvec iov[1];
- ssize_t n;
size_t reply_len = call->reply_len, len;
+ int ret;
rxrpc_kernel_set_tx_length(rxperf_socket, call->rxcall,
reply_len + sizeof(rxperf_magic_cookie));
@@ -512,13 +512,11 @@ static int rxperf_process_call(struct rxperf_call *call)
bvec_set_page(&bv, ZERO_PAGE(0), len, 0);
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);
- if (n < 0)
- return n;
- if (n == 0)
- return -EIO;
- reply_len -= n;
+ ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
+ rxperf_notify_end_reply_tx);
+ if (ret < 0)
+ return ret;
+ reply_len -= len;
}
len = sizeof(rxperf_magic_cookie);
@@ -526,16 +524,13 @@ static int rxperf_process_call(struct rxperf_call *call)
iov[0].iov_len = len;
iov_iter_kvec(&msg.msg_iter, WRITE, iov, 1, len);
msg.msg_flags = 0;
- n = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg, len,
- rxperf_notify_end_reply_tx);
- if (n >= 0)
- return 0; /* Success */
-
- if (n == -ENOMEM)
+ ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
+ rxperf_notify_end_reply_tx);
+ if (ret == -ENOMEM)
rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
RXGEN_SS_MARSHAL, -ENOMEM,
rxperf_abort_oom);
- return n;
+ return ret;
}
/*
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index fb8d48418882..393a2dcfda07 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -793,7 +793,6 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
* @sock: The socket the call is on
* @call: The call to send data through
* @msg: The data to send
- * @len: The amount of data to send
* @notify_end_tx: Notification that the last packet is queued.
*
* Allow a kernel service to send data on a call. The call must be in an state
@@ -804,8 +803,7 @@ int rxrpc_do_sendmsg(struct rxrpc_sock *rx, struct msghdr *msg, size_t len)
* Return: %0 if successful and a negative error code otherwise.
*/
int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
- struct msghdr *msg, size_t len,
- rxrpc_notify_end_tx_t notify_end_tx)
+ struct msghdr *msg, rxrpc_notify_end_tx_t notify_end_tx)
{
int ret;
@@ -814,12 +812,25 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
ASSERTCMP(msg->msg_name, ==, NULL);
ASSERTCMP(msg->msg_control, ==, NULL);
- mutex_lock(&call->user_mutex);
+ for (;;) {
+ mutex_lock(&call->user_mutex);
+
+ ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
+ msg_data_left(msg), notify_end_tx);
+ if (ret == -ESHUTDOWN)
+ ret = call->error;
+ if (ret < 0)
+ break;
+ if (msg_data_left(msg) == 0) {
+ ret = 0;
+ break;
+ }
+ if (ret == 0) {
+ ret = -EIO;
+ break;
+ }
+ }
- ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg, len,
- notify_end_tx);
- if (ret == -ESHUTDOWN)
- ret = call->error;
_leave(" = %d", ret);
return ret;
}
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 04/17] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()
2026-09-23 13:36 ` [PATCH net v11 04/17] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data() David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The patch adds this sentence to
Documentation/networking/rxrpc.rst: rxrpc_kernel_send_data() "returns…
Pre-existing issues:
- [Low] Documentation/networking/rxrpc.rst says the msg passed to
rxrpc_kernel_send_data() must not carry "any flags other than…
--
rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data()
This makes rxrpc_kernel_send_data() keep looping after a short send from
rxrpc_send_data() until the iterator is drained. It also drops the len
argument and uses the iterator count instead. On success the function now
returns 0 rather than the amount copied.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
> diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
> index 8926dab8e2e60..01ad12fdf3058 100644
> --- a/Documentation/networking/rxrpc.rst
> +++ b/Documentation/networking/rxrpc.rst
[ ... ]
> @@ -880,13 +879,16 @@ The kernel interface functions are as follows:
> MSG_MORE if there will be subsequent data sends for this call.
>
> The msg must not specify a destination address, control data or any flags
> - other than MSG_MORE. len is the total amount of data to transmit.
> + other than MSG_MORE.
[Severity: Low]
This is a pre-existing issue, and this patch did not introduce it. Is the
"any flags other than MSG_MORE" restriction still correct? The in-tree
caller afs_make_call() passes MSG_WAITALL:
fs/afs/rxrpc.c:afs_make_call() {
...
msg.msg_flags = MSG_WAITALL | (call->write_iter ? MSG_MORE : 0);
...
}
rxrpc_send_data() also acts on both MSG_DONTWAIT and MSG_WAITALL:
net/rxrpc/sendmsg.c:rxrpc_send_data() {
...
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
...
wait_for_space:
ret = -EAGAIN;
if (msg->msg_flags & MSG_DONTWAIT)
goto maybe_error;
...
ret = rxrpc_wait_for_tx_window(rx, call, &timeo,
msg->msg_flags & MSG_WAITALL);
...
}
This sentence is already being edited here. Could it be changed to list
the flags that are actually honoured? The same wording is still present
at the end of the series.
>
> notify_end_rx can be NULL or it can be used to specify a function to be
> called when the call changes state to end the Tx phase. This function is
> called with a spinlock held to prevent the last DATA packet from being
> transmitted until the function returns.
>
> + It returns 0 if all the data is queued and a negative error code on
> + failure.
[Severity: Low]
Is "queued" the right word here? Suppose MSG_MORE is set and the data
does not exactly fill a txbuf. In that case rxrpc_send_data() leaves the
partly filled txbuf in call->tx_pending and does not call
rxrpc_queue_packet():
net/rxrpc/sendmsg.c:rxrpc_send_data() {
...
/* add the packet to the send queue if it's now full */
if (!txb->space ||
(msg_data_left(msg) == 0 && !more)) {
...
rxrpc_queue_packet(rx, call, txb, notify_end_tx);
call->tx_pending = NULL;
}
...
}
At that point the iterator is empty, so the new loop in
rxrpc_kernel_send_data() returns 0:
if (msg_data_left(msg) == 0) {
ret = 0;
break;
}
Two examples of this path are rxperf_process_call() sending ZERO_PAGE
chunks, and afs_make_call() sending the request header before
write_iter.
So a return of 0 seems to mean the data was taken from the iterator and
buffered, not queued in the rxrpc_queue_packet() sense. A later patch in
the series, "rxrpc: Fix sendmsg length", uses the word "buffered" for
this same state in this document.
A negative return can also now follow a partial transfer. An earlier pass
of the loop may consume part of the iterator before a later pass fails.
Should the documentation say so as well?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 05/17] afs: Fix afs to abort the rxrpc call on send error
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (3 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 04/17] rxrpc: Fix lack of short-send handling in rxrpc_kernel_send_data() David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:36 ` [PATCH net v11 06/17] rxrpc: Fix aborting in rxperf test server David Howells
` (12 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
Fix afs_send_empty_reply() and afs_send_simple_reply() to always try to
abort the rxrpc call rather than just aborting on -ENOMEM and otherwise
abandoning it. If the call is already complete due to network failure or a
received abort, this will do nothing.
Also make afs_make_call() always abort on send error; again, it does
nothing if the rxrpc call is already dead.
Fixes: 08e0e7c82eea ("[AF_RXRPC]: Make the in-kernel AFS filesystem use AF_RXRPC.")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
fs/afs/rxrpc.c | 36 +++++++++++-------------------------
include/trace/events/rxrpc.h | 2 +-
2 files changed, 12 insertions(+), 26 deletions(-)
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index bcb8bb15d471..c38458157ecc 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -441,10 +441,8 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
return;
error_do_abort:
- if (ret != -ECONNABORTED)
- rxrpc_kernel_abort_call(call->net->socket, rxcall,
- RX_USER_ABORT, ret,
- afs_abort_send_data_error);
+ rxrpc_kernel_abort_call(call->net->socket, rxcall,
+ RX_USER_ABORT, ret, afs_abort_send_data_error);
if (call->async) {
afs_see_call(call, afs_call_trace_async_abort);
return;
@@ -857,6 +855,7 @@ void afs_send_empty_reply(struct afs_call *call)
{
struct afs_net *net = call->net;
struct msghdr msg;
+ int ret;
_enter("");
@@ -869,22 +868,12 @@ void afs_send_empty_reply(struct afs_call *call)
msg.msg_controllen = 0;
msg.msg_flags = 0;
- switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
- afs_notify_end_reply_tx)) {
- case 0:
- _leave(" [replied]");
- return;
-
- case -ENOMEM:
- _debug("oom");
+ ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
+ afs_notify_end_reply_tx);
+ if (ret < 0)
rxrpc_kernel_abort_call(net->socket, call->rxcall,
- RXGEN_SS_MARSHAL, -ENOMEM,
- afs_abort_oom);
- fallthrough;
- default:
- _leave(" [error]");
- return;
- }
+ RXGEN_SS_MARSHAL, ret,
+ afs_abort_send_error);
}
/*
@@ -913,12 +902,9 @@ void afs_send_simple_reply(struct afs_call *call, const void *buf, size_t len)
ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
afs_notify_end_reply_tx);
if (ret < 0) {
- if (ret == -ENOMEM) {
- _debug("oom");
- rxrpc_kernel_abort_call(net->socket, call->rxcall,
- RXGEN_SS_MARSHAL, -ENOMEM,
- afs_abort_oom);
- }
+ rxrpc_kernel_abort_call(net->socket, call->rxcall,
+ RXGEN_SS_MARSHAL, ret,
+ afs_abort_send_error);
_leave(" [error]");
}
}
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 704a10de6670..554dfb777b93 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -20,10 +20,10 @@
/* AFS errors */ \
EM(afs_abort_general_error, "afs-error") \
EM(afs_abort_interrupted, "afs-intr") \
- EM(afs_abort_oom, "afs-oom") \
EM(afs_abort_op_not_supported, "afs-op-notsupp") \
EM(afs_abort_probeuuid_negative, "afs-probeuuid-neg") \
EM(afs_abort_send_data_error, "afs-send-data") \
+ EM(afs_abort_send_error, "afs-send-error") \
EM(afs_abort_unmarshal_error, "afs-unmarshal") \
EM(afs_abort_unsupported_sec_class, "afs-unsup-sec-class") \
/* rxperf errors */ \
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 05/17] afs: Fix afs to abort the rxrpc call on send error
2026-09-23 13:36 ` [PATCH net v11 05/17] afs: Fix afs to abort the rxrpc call on send error David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] When sending a reply fails on an incoming cache-manager (CB)
service call, the afs_call is never completed.
--
afs: Fix afs to abort the rxrpc call on send error
This patch makes afs_send_empty_reply() and afs_send_simple_reply() abort
the rxrpc call on any send error, not only on -ENOMEM. It also makes
afs_make_call() abort on any send error. The afs_abort_oom trace reason is
replaced with afs_abort_send_error.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
> diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
> index bcb8bb15d471d..c38458157eccf 100644
> --- a/fs/afs/rxrpc.c
> +++ b/fs/afs/rxrpc.c
[ ... ]
> @@ -869,22 +868,12 @@ void afs_send_empty_reply(struct afs_call *call)
> msg.msg_controllen = 0;
> msg.msg_flags = 0;
>
> - switch (rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
> - afs_notify_end_reply_tx)) {
> - case 0:
> - _leave(" [replied]");
> - return;
> -
> - case -ENOMEM:
> - _debug("oom");
> + ret = rxrpc_kernel_send_data(net->socket, call->rxcall, &msg,
> + afs_notify_end_reply_tx);
> + if (ret < 0)
> rxrpc_kernel_abort_call(net->socket, call->rxcall,
> - RXGEN_SS_MARSHAL, -ENOMEM,
> - afs_abort_oom);
> - fallthrough;
> - default:
> - _leave(" [error]");
> - return;
> - }
> + RXGEN_SS_MARSHAL, ret,
> + afs_abort_send_error);
> }
[Severity: High]
Is the afs_call still abandoned here when the send fails? The rxrpc call
is aborted, but nothing calls afs_set_call_complete(), so the afs_call
stays in AFS_CALL_SV_REPLYING. afs_send_simple_reply() has the same
pattern.
The commit message says the change stops "otherwise abandoning it". That
is true for the rxrpc call, but it does not seem to hold for the afs_call.
The only way out of SV_REPLYING is afs_notify_end_reply_tx():
afs_set_call_state(call, AFS_CALL_SV_REPLYING, AFS_CALL_SV_AWAIT_ACK);
That callback only runs once the final DATA packet has been queued, and
that does not happen on a send error.
A later notification (from this abort or from a received abort) goes
through afs_wake_up_async_call() -> afs_process_async_call() ->
afs_deliver_to_call(). The loop there does not handle SV_REPLYING:
while (state = READ_ONCE(call->state),
state == AFS_CALL_CL_AWAIT_REPLY ||
state == AFS_CALL_SV_AWAIT_OP_ID ||
state == AFS_CALL_SV_AWAIT_REQUEST ||
state == AFS_CALL_SV_AWAIT_ACK
) {
None of the CB call types has a ->done hook. So the drop_ref reference
set by afs_charge_preallocation() is never released, because only
afs_set_call_complete() drops it.
If that's right, the afs_call leaks along with its rxrpc_call, peer and
server references and its buffers. net->nr_outstanding_calls would then
never reach zero, and afs_close_socket() would block forever here during
netns teardown or module unload:
wait_var_event(&net->nr_outstanding_calls,
!atomic_read(&net->nr_outstanding_calls));
A remote peer seems able to trigger this. It can send CB.Probe or
CB.CallBack and abort right after the last request packet. Then
SRXAFSCB_Probe() -> afs_send_empty_reply() -> rxrpc_kernel_send_data()
returns -ESHUTDOWN, and rxrpc_kernel_abort_call() does nothing.
-ENOMEM from txbuf or txqueue allocation leads to the same state.
afs_abort_service_call() in fs/afs/cmservice.c already pairs the two
steps:
rxrpc_kernel_abort_call(call->net->socket, call->rxcall,
abort_code, error, why);
afs_set_call_complete(call, error, 0);
Should the send-error paths in afs_send_empty_reply() and
afs_send_simple_reply() also call afs_set_call_complete(call, ret, 0)
after the abort? No later patch in the series changes these two
functions.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 06/17] rxrpc: Fix aborting in rxperf test server
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (4 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 05/17] afs: Fix afs to abort the rxrpc call on send error David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 07/17] rxrpc: Fix sendmsg length David Howells
` (11 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel
Fix rxperf_process_call() to always abort if it gets a send error rather
than only aborting on ENOMEM.
Fixes: 75bfdbf2fca3 ("rxrpc: Implement an in-kernel rxperf server for testing purposes")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
include/trace/events/rxrpc.h | 2 +-
net/rxrpc/rxperf.c | 11 ++++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 554dfb777b93..56dc9b614071 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -28,8 +28,8 @@
EM(afs_abort_unsupported_sec_class, "afs-unsup-sec-class") \
/* rxperf errors */ \
EM(rxperf_abort_general_error, "rxperf-error") \
- EM(rxperf_abort_oom, "rxperf-oom") \
EM(rxperf_abort_op_not_supported, "rxperf-op-notsupp") \
+ EM(rxperf_abort_send_error, "rxperf-send-error") \
EM(rxperf_abort_unmarshal_error, "rxperf-unmarshal") \
/* RxKAD security errors */ \
EM(rxkad_abort_1_short_check, "rxkad1-short-check") \
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 981c0596c774..823eedc5d16f 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -515,7 +515,7 @@ static int rxperf_process_call(struct rxperf_call *call)
ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
rxperf_notify_end_reply_tx);
if (ret < 0)
- return ret;
+ goto send_error;
reply_len -= len;
}
@@ -526,10 +526,11 @@ static int rxperf_process_call(struct rxperf_call *call)
msg.msg_flags = 0;
ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
rxperf_notify_end_reply_tx);
- if (ret == -ENOMEM)
- rxrpc_kernel_abort_call(rxperf_socket, call->rxcall,
- RXGEN_SS_MARSHAL, -ENOMEM,
- rxperf_abort_oom);
+ if (ret == 0)
+ return 0;
+send_error:
+ rxrpc_kernel_abort_call(rxperf_socket, call->rxcall, RXGEN_SS_MARSHAL,
+ ret, rxperf_abort_send_error);
return ret;
}
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 07/17] rxrpc: Fix sendmsg length
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (5 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 06/17] rxrpc: Fix aborting in rxperf test server David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 08/17] rxrpc: Fix error handling in rxrpc_send_data() David Howells
` (10 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman
rxrpc_send_data() is given two data lengths (len and msg->msg_iter.count)
and is inconsistent about how it uses them. Fix this by using len in
preference to msg->msg_iter.count. Also limit the amount copied to either
len or msg->msg_iter.count, whichever is smaller.
Note that, currently, all the callers have len and msg->msg_iter.count the
same and so the problem won't occur. This is a prerequisite for another
patch that fixes the handling of encryption errors.
Fixes: 382d7974de31 ("RxRPC: Use iov_iter_count() in rxrpc_send_data() instead of the len argument")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
net/rxrpc/sendmsg.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 393a2dcfda07..312be27ca75b 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -374,9 +374,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;
}
@@ -402,7 +402,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);
call->tx_pending = NULL;
}
- } while (msg_data_left(msg) > 0);
+ } while (len > 0 && msg_data_left(msg) > 0);
success:
ret = copied;
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 08/17] rxrpc: Fix error handling in rxrpc_send_data()
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (6 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 07/17] rxrpc: Fix sendmsg length David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:36 ` [PATCH net v11 09/17] rxrpc: Fix packet encryption error handling David Howells
` (9 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman, stable
Fix the error handling in rxrpc_send_data() so that it doesn't return an
error if it has successfully queued the last packet of a call, but the call
has seen to have completed after it did that. Rather, leave it to
recvmsg() to report the completion (which it will do anyway).
The problem with trying to report the error twice is that the caller may
try to clean up the dead call twice.
Further, if we haven't queued the final packet yet, return -ESHUTDOWN if
the call is now marked complete (e.g. it got aborted by the peer) as
there's no point sendmsg() continuing to try to add data to a call if it is
defunct. The application should abort the call and then call recvmsg() to
pick up the reason.
Fixes: 4ba68c519255 ("rxrpc: Return an error to sendmsg if call failed")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
fs/afs/rxrpc.c | 11 +++--
net/rxrpc/rxperf.c | 29 +++++++----
net/rxrpc/sendmsg.c | 114 ++++++++++++++++++++++++++++++--------------
3 files changed, 103 insertions(+), 51 deletions(-)
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index c38458157ecc..0b9027549580 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -448,13 +448,14 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
return;
}
- if (ret == -ECONNABORTED) {
+ if (ret == -ESHUTDOWN) {
len = 0;
iov_iter_kvec(&msg.msg_iter, ITER_DEST, NULL, 0, 0);
- rxrpc_kernel_recv_data(call->net->socket, rxcall,
- &msg.msg_iter, &len, false,
- &call->abort_code, &call->service_id);
- call->responded = true;
+ ret = rxrpc_kernel_recv_data(call->net->socket, rxcall,
+ &msg.msg_iter, &len, false,
+ &call->abort_code, &call->service_id);
+ if (ret == -ECONNABORTED)
+ call->responded = true;
}
call->error = ret;
trace_afs_call_done(call);
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 823eedc5d16f..5042e8bfca55 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -74,7 +74,7 @@ static struct workqueue_struct *rxperf_workqueue;
static void rxperf_deliver_to_call(struct work_struct *work);
static int rxperf_deliver_param_block(struct rxperf_call *call);
static int rxperf_deliver_request(struct rxperf_call *call);
-static int rxperf_process_call(struct rxperf_call *call);
+static void rxperf_process_call(struct rxperf_call *call);
static void rxperf_charge_preallocation(struct work_struct *work);
static DECLARE_WORK(rxperf_charge_preallocation_work,
@@ -293,18 +293,28 @@ static void rxperf_deliver_to_call(struct work_struct *work)
state == RXPERF_CALL_SV_AWAIT_ACK
) {
if (state == RXPERF_CALL_SV_AWAIT_ACK) {
- if (!rxrpc_kernel_check_life(rxperf_socket, call->rxcall))
+ size_t len = 0;
+ iov_iter_kvec(&call->iter, ITER_DEST, NULL, 0, 0);
+ ret = rxrpc_kernel_recv_data(rxperf_socket,
+ call->rxcall, &call->iter,
+ &len, false, &remote_abort,
+ &call->service_id);
+
+ if (ret == -EINPROGRESS || ret == -EAGAIN)
+ return;
+ if (ret < 0 || ret == 1) {
+ if (ret == 1)
+ ret = 0;
goto call_complete;
+ }
return;
}
ret = call->deliver(call);
- if (ret == 0)
- ret = rxperf_process_call(call);
-
switch (ret) {
case 0:
- continue;
+ rxperf_process_call(call);
+ return;
case -EINPROGRESS:
case -EAGAIN:
return;
@@ -496,7 +506,7 @@ static int rxperf_deliver_request(struct rxperf_call *call)
/*
* Process a call for which we've received the request.
*/
-static int rxperf_process_call(struct rxperf_call *call)
+static void rxperf_process_call(struct rxperf_call *call)
{
struct msghdr msg = {};
struct bio_vec bv;
@@ -527,11 +537,11 @@ static int rxperf_process_call(struct rxperf_call *call)
ret = rxrpc_kernel_send_data(rxperf_socket, call->rxcall, &msg,
rxperf_notify_end_reply_tx);
if (ret == 0)
- return 0;
+ return;
+
send_error:
rxrpc_kernel_abort_call(rxperf_socket, call->rxcall, RXGEN_SS_MARSHAL,
ret, rxperf_abort_send_error);
- return ret;
}
/*
@@ -684,4 +694,3 @@ static void __exit rxperf_exit(void)
rcu_barrier();
}
module_exit(rxperf_exit);
-
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 312be27ca75b..80c6afc64787 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -324,19 +324,10 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
__releases(&call->user_mutex)
{
struct sock *sk = &rx->sk;
- enum rxrpc_call_state state;
long timeo;
bool more = msg->msg_flags & MSG_MORE;
int ret, copied = 0;
- if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
- trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
- call->cid, call->call_id, call->rx_consumed,
- 0, -EPROTO);
- ret = -EPROTO;
- goto out_unlock;
- }
-
timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
ret = rxrpc_wait_to_be_connected(call, &timeo);
@@ -355,21 +346,31 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
reload:
ret = -EPIPE;
if (sk->sk_shutdown & SEND_SHUTDOWN)
- goto maybe_error;
- state = rxrpc_call_state(call);
- ret = -ESHUTDOWN;
- if (state >= RXRPC_CALL_COMPLETE)
- goto maybe_error;
- ret = -EPROTO;
- if (state != RXRPC_CALL_CLIENT_PRE_SEND &&
- state != RXRPC_CALL_CLIENT_SEND_REQUEST &&
- state != RXRPC_CALL_SERVER_ACK_REQUEST &&
- state != RXRPC_CALL_SERVER_SEND_REPLY) {
- /* Request phase complete for this client call */
+ goto out_unlock;
+
+ switch (rxrpc_call_state(call)) {
+ case RXRPC_CALL_CLIENT_PRE_SEND:
+ case RXRPC_CALL_CLIENT_SEND_REQUEST:
+ case RXRPC_CALL_SERVER_ACK_REQUEST:
+ case RXRPC_CALL_SERVER_SEND_REPLY:
+ break;
+ case RXRPC_CALL_COMPLETE:
+ ret = -ESHUTDOWN;
+ goto out_unlock;
+ default:
+ ret = -EPROTO;
trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
call->cid, call->call_id, call->rx_consumed,
0, -EPROTO);
- goto maybe_error;
+ goto out_unlock;
+ }
+
+ if (unlikely(test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags))) {
+ trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_late_send,
+ call->cid, call->call_id, call->rx_consumed,
+ 0, -EPROTO);
+ ret = -EPROTO;
+ goto out_unlock;
}
ret = -EMSGSIZE;
@@ -435,8 +436,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
/* check for the far side aborting the call or a network error
* occurring */
+ ret = -ESHUTDOWN;
if (rxrpc_call_is_complete(call))
- goto call_terminated;
+ goto out_unlock;
/* add the packet to the send queue if it's now full */
if (!txb->space ||
@@ -449,31 +451,71 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
goto out_unlock;
rxrpc_queue_packet(rx, call, txb, notify_end_tx);
call->tx_pending = NULL;
+
+ /* At this point, if that was the last packet, it may
+ * have been transmitted and the reply (client call) or
+ * final ACK (service call) may have been received,
+ * completing the call.
+ */
}
} while (len > 0 && msg_data_left(msg) > 0);
-success:
+ /* Don't check for call completeness here, but leave that to recvmsg or
+ * a further call to sendmsg().
+ */
ret = copied;
- if (rxrpc_call_is_complete(call) &&
- call->error < 0)
- ret = call->error;
out_unlock:
mutex_unlock(&call->user_mutex);
+out:
+
+ /* The return value is a bit complicated as we want to avoid returning
+ * an error if we have queued the final packet. In descending order of
+ * preference:
+ *
+ * (1) If the send side of the socket is shut down, -EPIPE.
+ *
+ * (2) If the call has terminated early, likely due to an external
+ * event such as being remotely aborted: -ESHUTDOWN.
+ *
+ * (3) If the call is in the wrong state to transmit: -EPROTO.
+ *
+ * (4) If another sendmsg() has already queued the last packet: -EPROTO.
+ *
+ * (5) If we queue the last packet: the amount copied (which may be
+ * zero). recvmsg() should be used to collect the result.
+ *
+ * (6) If some data has been copied by this call: the amount copied
+ * (which will be greater than zero).
+ *
+ * (7) Any other error.
+ *
+ * For (1)-(4), there's no point in continuing with the sendmsg(). The
+ * app should abort the call (just in case the error came from
+ * somewhere else) and then use recvmsg() to collect the final result
+ * of the call.
+ */
_leave(" = %d", ret);
return ret;
-call_terminated:
- ret = call->error;
- goto out_unlock;
-
maybe_error:
- if (copied)
- goto success;
+ if (copied) {
+ if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
+ /* If we've get here, we must have slept waiting for space and .
+ */
+ ret = copied;
+ goto out_unlock;
+ }
+ if (rxrpc_call_is_complete(call)) {
+ ret = -ESHUTDOWN;
+ goto out_unlock;
+ }
+ ret = copied;
+ }
goto out_unlock;
efault:
ret = -EFAULT;
- goto out_unlock;
+ goto maybe_error;
wait_for_space:
ret = -EAGAIN;
@@ -496,7 +538,9 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
goto reload;
out_nolock:
_leave(" = %d [intr]", ret);
- return copied ?: ret;
+ if (copied)
+ ret = copied;
+ goto out;
}
/*
@@ -818,8 +862,6 @@ int rxrpc_kernel_send_data(struct socket *sock, struct rxrpc_call *call,
ret = rxrpc_send_data(rxrpc_sk(sock->sk), call, msg,
msg_data_left(msg), notify_end_tx);
- if (ret == -ESHUTDOWN)
- ret = call->error;
if (ret < 0)
break;
if (msg_data_left(msg) == 0) {
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 08/17] rxrpc: Fix error handling in rxrpc_send_data()
2026-09-23 13:36 ` [PATCH net v11 08/17] rxrpc: Fix error handling in rxrpc_send_data() David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 2 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The fix leaves the new -ESHUTDOWN contract incomplete, and this
breaks AFS abort handling.
- [Medium] At this commit, the new SV_AWAIT_ACK handling in
rxperf_deliver_to_call() treats a return of 1 from…
- [Medium] rxperf_process_call() now returns void, and
rxperf_deliver_to_call() returns right after calling it (`case 0:…
- [Low] In rxrpc_send_data(), the new `test_bit(RXRPC_CALL_TX_NO_MORE)`
branch under maybe_error is dead code, and its comment stops mid-…
--
rxrpc: Fix error handling in rxrpc_send_data()
This commit changes rxrpc_send_data() so that it doesn't return an error
after it has queued the last packet of a call. It also makes it return
-ESHUTDOWN if the call completes before the final packet is queued. The
-ESHUTDOWN to call->error translation is removed from
rxrpc_kernel_send_data(), and afs_make_call() and rxperf are changed to
match.
> diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
> index c38458157eccf..0b9027549580e 100644
> --- a/fs/afs/rxrpc.c
> +++ b/fs/afs/rxrpc.c
> @@ -448,13 +448,14 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
> return;
> }
>
> - if (ret == -ECONNABORTED) {
> + if (ret == -ESHUTDOWN) {
> len = 0;
> iov_iter_kvec(&msg.msg_iter, ITER_DEST, NULL, 0, 0);
> - rxrpc_kernel_recv_data(call->net->socket, rxcall,
> - &msg.msg_iter, &len, false,
> - &call->abort_code, &call->service_id);
> - call->responded = true;
> + ret = rxrpc_kernel_recv_data(call->net->socket, rxcall,
> + &msg.msg_iter, &len, false,
> + &call->abort_code, &call->service_id);
> + if (ret == -ECONNABORTED)
> + call->responded = true;
> }
[Severity: High]
Can rxrpc_kernel_send_data() still return -ECONNABORTED for a call that
the peer has aborted? The translation from -ESHUTDOWN to call->error is
gone. However, some paths in rxrpc_send_data() still return the raw
call->error and never reach the new RXRPC_CALL_COMPLETE case at reload.
rxrpc_wait_to_be_connected() still does:
no_wait:
if (ret == 0 && rxrpc_call_is_complete(call))
ret = call->error;
rxrpc_send_data() then goes straight to out_unlock with that value.
The rxrpc_wait_for_tx_window_*() helpers also still do:
if (rxrpc_call_is_complete(call))
return call->error;
That value goes to out_nolock. out_nolock returns copied if it is
nonzero, and the raw error otherwise.
Here is one example. The client is blocked in
rxrpc_wait_for_tx_window_nonintr() during a StoreData, and the fileserver
aborts the call with VBUSY or VOFFLINE:
- out_nolock returns copied (> 0).
- rxrpc_kernel_send_data() loops.
- The next rxrpc_send_data() gets -ECONNABORTED from
rxrpc_wait_to_be_connected().
In that case the check above fails and rxrpc_kernel_recv_data() is not
called. So call->abort_code stays 0 and call->responded stays false.
afs_select_fileserver() then sees -ECONNABORTED with an abort code of 0
and takes the default case. Doesn't that lose the VBUSY/VOFFLINE retry,
the VMOVED handling and the EDQUOT/ENOSPC translation?
Before this patch, those paths all returned -ECONNABORTED, which the old
check here handled.
For userspace AF_RXRPC, an early termination can now show up three ways:
-ESHUTDOWN, the raw call->error, or a positive count from out_nolock.
That doesn't seem to match the new return-value comment in
rxrpc_send_data() or the commit message:
"return -ESHUTDOWN if the call is now marked complete (e.g. it got
aborted by the peer)"
Should these helpers return -ESHUTDOWN for a completed call instead, or
should rxrpc_kernel_send_data() keep the translation?
[ ... ]
> diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
> index 823eedc5d16f9..5042e8bfca550 100644
> --- a/net/rxrpc/rxperf.c
> +++ b/net/rxrpc/rxperf.c
[ ... ]
> @@ -293,18 +293,28 @@ static void rxperf_deliver_to_call(struct work_struct *work)
> state == RXPERF_CALL_SV_AWAIT_ACK
> ) {
> if (state == RXPERF_CALL_SV_AWAIT_ACK) {
> - if (!rxrpc_kernel_check_life(rxperf_socket, call->rxcall))
> + size_t len = 0;
> + iov_iter_kvec(&call->iter, ITER_DEST, NULL, 0, 0);
> + ret = rxrpc_kernel_recv_data(rxperf_socket,
> + call->rxcall, &call->iter,
> + &len, false, &remote_abort,
> + &call->service_id);
> +
> + if (ret == -EINPROGRESS || ret == -EAGAIN)
> + return;
> + if (ret < 0 || ret == 1) {
> + if (ret == 1)
> + ret = 0;
> goto call_complete;
> + }
[Severity: Medium]
Does a return of 1 here really mean the call has completed? At this
commit, rxrpc_kernel_recv_data() returns 1 from read_phase_complete once
the request data has been fully read, which can be before the final ACK
arrives:
read_phase_complete:
ret = 1;
Suppose a requeued work item runs in RXPERF_CALL_SV_AWAIT_ACK while the
reply is still in flight. Wouldn't call_complete then call
rxrpc_kernel_shutdown_call(), rxrpc_kernel_put_call() and kfree(call) on
a call that is still live?
Also, rxrpc_kernel_recv_data() doesn't seem to return -EINPROGRESS, so
that test never matches.
A later commit in the series, "rxrpc: Fix return in rxrpc_recvmsg_data()
for service calls", fixes this. It makes rxrpc_kernel_recv_data() return
2 for a service call that hasn't completed, and changes this test to
check for 2. Could that change be folded in or moved earlier, so that
this commit doesn't have the problem on its own?
> return;
> }
>
> ret = call->deliver(call);
> - if (ret == 0)
> - ret = rxperf_process_call(call);
> -
> switch (ret) {
> case 0:
> - continue;
> + rxperf_process_call(call);
> + return;
[Severity: Medium]
Can this leak the rxperf_call and its rxrpc_call reference if sending the
reply fails? Before this patch, a send error went to the default case and
then to call_complete. Now teardown relies on a later notify_rx
requeueing the work.
rxperf_close_socket() does:
kernel_listen(rxperf_socket, 0);
kernel_sock_shutdown(rxperf_socket, SHUT_RDWR);
flush_workqueue(rxperf_workqueue);
sock_release(rxperf_socket);
Suppose a work item is inside rxperf_process_call(), sending a
multi-page reply, when the shutdown happens. The next chunk hits
SEND_SHUTDOWN at reload in rxrpc_send_data() and gets -EPIPE.
rxperf_process_call() then aborts the call and returns.
The completion then goes through __rxrpc_notify_socket(), which skips
notify_rx once the socket is closed:
if (rx && sk->sk_state < RXRPC_CLOSE) {
if (call->notify_rx) {
So the work is never requeued, and neither kfree(call) nor
rxrpc_kernel_put_call() runs. Wouldn't rxrpc_destroy_all_calls() then
block in wait_var_event() on rxnet->nr_calls when rxrpc is unloaded?
[ ... ]
> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index 312be27ca75b5..80c6afc647870 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c
[ ... ]
> @@ -449,31 +451,71 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
[ ... ]
> maybe_error:
> - if (copied)
> - goto success;
> + if (copied) {
> + if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
> + /* If we've get here, we must have slept waiting for space and .
> + */
> + ret = copied;
> + goto out_unlock;
> + }
[Severity: Low]
This isn't a bug, but the comment stops mid-sentence ("waiting for space
and ."), and "we've get" should probably be "we've got".
Can this branch be reached at all? Only rxrpc_queue_packet() sets
RXRPC_CALL_TX_NO_MORE, and only for the last packet. That happens only
when len == 0 && !more, which ends the loop at "ret = copied". After a
sleep, reload rejects TX_NO_MORE with -EPROTO before anything can jump
to maybe_error.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 09/17] rxrpc: Fix packet encryption error handling
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (7 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 08/17] rxrpc: Fix error handling in rxrpc_send_data() David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:36 ` [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls David Howells
` (8 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
In rxrpc_send_data(), if ->secure_packet() returns an error, the code
currently just jumps to out: and returns the error to the app on the
assumption that any error returned by this is automatically fatal for the
call, and may even have corrupted the transmission queue - but leaving it
to userspace to deal with. Nothing stops the application from retrying the
sendmsg(), which will try to encrypt the buffer again, and might succeed
with a corrupt buffer.
Fix rxrpc_send_data() in the following ways:
(1) If -ENOMEM is returned, assume we never got as far as the encryption
and that the operation is retryable. In which case, jump to
maybe_error_rewind and, if we've copied all remaining data into the
last packet, remove some of the bytes from it that we just added so
that we don't tell the caller that we've completed the transmission
phase. The iterator is also correspondingly rewound.
(2) If any other error occurs, set the TX_ERROR flag on the call and
return that error directly; on all subsequent attempts to add data to
the call, return -EIO. The app must then abort the call to get rid of
it (this allows the app to choose the abort code to use).
Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
Documentation/networking/rxrpc.rst | 6 ++-
include/trace/events/rxrpc.h | 1 +
net/rxrpc/ar-internal.h | 1 +
net/rxrpc/sendmsg.c | 68 +++++++++++++++++++++++++-----
4 files changed, 63 insertions(+), 13 deletions(-)
diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
index 01ad12fdf305..9b7eeae84148 100644
--- a/Documentation/networking/rxrpc.rst
+++ b/Documentation/networking/rxrpc.rst
@@ -878,8 +878,10 @@ The kernel interface functions are as follows:
exclusively to in-kernel virtual addresses. msg.msg_flags may be given
MSG_MORE if there will be subsequent data sends for this call.
- The msg must not specify a destination address, control data or any flags
- other than MSG_MORE.
+ msg must not specify a destination address, control data or any flags
+ other than MSG_MORE. The last-packet flag will only be set on the
+ outgoing packet if MSG_MORE is not set and all the data in the iterator is
+ buffered.
notify_end_rx can be NULL or it can be used to specify a function to be
called when the call changes state to end the Tx phase. This function is
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 56dc9b614071..a5c92592d8f9 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -148,6 +148,7 @@
EM(rxrpc_eproto_wrong_security, "wrong-sec") \
EM(rxrpc_recvmsg_excess_data, "recvmsg-excess") \
EM(rxrpc_recvmsg_short_data, "recvmsg-short") \
+ EM(rxrpc_sendmsg_tx_error, "tx-error") \
E_(rxrpc_sendmsg_late_send, "sendmsg-late")
#define rxrpc_call_poke_traces \
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 865f05fe37ab..a6f830c1621f 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -642,6 +642,7 @@ enum rxrpc_call_flag {
RXRPC_CALL_TX_LAST, /* Last packet in Tx buffer (at rxtx_top) */
RXRPC_CALL_TX_ALL_ACKED, /* Last packet has been hard-acked */
RXRPC_CALL_TX_NO_MORE, /* No more data to transmit (MSG_MORE deasserted) */
+ RXRPC_CALL_TX_ERROR, /* Terminal error; call needs abort */
RXRPC_CALL_SEND_PING, /* A ping will need to be sent */
RXRPC_CALL_RETRANS_TIMEOUT, /* Retransmission due to timeout occurred */
RXRPC_CALL_BEGAN_RX_TIMER, /* We began the expect_rx_by timer */
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 80c6afc64787..e3b6347888bf 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -324,6 +324,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
__releases(&call->user_mutex)
{
struct sock *sk = &rx->sk;
+ unsigned int rewind_by = 0;
long timeo;
bool more = msg->msg_flags & MSG_MORE;
int ret, copied = 0;
@@ -372,6 +373,13 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
ret = -EPROTO;
goto out_unlock;
}
+ if (unlikely(test_bit(RXRPC_CALL_TX_ERROR, &call->flags))) {
+ trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_tx_error,
+ call->cid, call->call_id, call->rx_consumed,
+ 0, -EIO);
+ ret = -EIO;
+ goto out_unlock;
+ }
ret = -EMSGSIZE;
if (call->tx_total_len != -1) {
@@ -425,6 +433,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
copy, &msg->msg_iter))
goto efault;
_debug("added");
+ rewind_by = copy;
txb->space -= copy;
txb->len += copy;
txb->offset += copy;
@@ -443,14 +452,29 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
/* add the packet to the send queue if it's now full */
if (!txb->space ||
(len == 0 && !more)) {
- if (len == 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) {
+ /* Assume that ENOMEM here means that the
+ * encryption hasn't happened yet. The data is
+ * aligned to avoid the need for slow buffering
+ * in the crypto walk.
+ */
+ if (ret == -ENOMEM)
+ goto maybe_error_rewind;
+ set_bit(RXRPC_CALL_TX_ERROR, &call->flags);
goto out_unlock;
+ }
+
+ if (len == 0 && !more)
+ txb->flags |= RXRPC_LAST_PACKET;
rxrpc_queue_packet(rx, call, txb, notify_end_tx);
call->tx_pending = NULL;
+ rewind_by = 0;
/* At this point, if that was the last packet, it may
* have been transmitted and the reply (client call) or
@@ -481,22 +505,44 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
*
* (4) If another sendmsg() has already queued the last packet: -EPROTO.
*
- * (5) If we queue the last packet: the amount copied (which may be
+ * (5) If an error occurs that may have corrupted the transmission
+ * buffer (e.g. crypto failure) or unusable crypto was encountered:
+ * the error given (and RXRPC_CALL_TX_ERROR is set to cause -EIO to
+ * be returned from further calls).
+ *
+ * (6) If we queue the last packet: the amount copied (which may be
* zero). recvmsg() should be used to collect the result.
*
- * (6) If some data has been copied by this call: the amount copied
+ * (7) If some data has been copied by this call: the amount copied
* (which will be greater than zero).
*
- * (7) Any other error.
+ * (8) Any other error.
*
- * For (1)-(4), there's no point in continuing with the sendmsg(). The
- * app should abort the call (just in case the error came from
- * somewhere else) and then use recvmsg() to collect the final result
- * of the call.
+ * For (1)-(6), there's no point in continuing with the sendmsg() and
+ * we no longer care how much has been queued as the call is no longer
+ * viable. The app should abort the call (just in case the error came
+ * from somewhere else) and then use recvmsg() to collect the final
+ * result of the call.
*/
_leave(" = %d", ret);
return ret;
+maybe_error_rewind:
+ /* If we got a retryable error after copying all the supplied data into
+ * the last packet, we need to rewind as much as we can so the caller
+ * knows they need to retry the sendmsg.
+ */
+ if (rewind_by && !more && !len) {
+ struct rxrpc_txbuf *txb = call->tx_pending;
+
+ txb->space += rewind_by;
+ txb->len -= rewind_by;
+ txb->offset -= rewind_by;
+ copied -= rewind_by;
+ if (call->tx_total_len != -1)
+ call->tx_total_len += rewind_by;
+ iov_iter_revert(&msg->msg_iter, rewind_by);
+ }
maybe_error:
if (copied) {
if (test_bit(RXRPC_CALL_TX_NO_MORE, &call->flags)) {
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 09/17] rxrpc: Fix packet encryption error handling
2026-09-23 13:36 ` [PATCH net v11 09/17] rxrpc: Fix packet encryption error handling David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The patch renumbers the return-value comment block in
rxrpc_send_data() (net/rxrpc/sendmsg.c) and adds a new item (5) for…
- [Low] The patch adds a new userspace-visible sendmsg() behaviour on
AF_RXRPC sockets that Documentation/networking/rxrpc.rst never…
Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but the
rxrpc_kernel_send_data documentation block the patch edits…
--
rxrpc: Fix packet encryption error handling
This changes rxrpc_send_data() so that -ENOMEM from ->secure_packet() is
treated as retryable. The data just added to the last packet is removed
again and the iterator is rewound. Any other encryption error sets a new
sticky RXRPC_CALL_TX_ERROR flag, and after that every data send on the
call returns -EIO until the application aborts the call.
Closes: https://sashiko.dev/#/patchset/20260702144919.172295-1-dhowells%40redhat.com
> diff --git a/Documentation/networking/rxrpc.rst b/Documentation/networking/rxrpc.rst
> index 01ad12fdf3058..9b7eeae841485 100644
> --- a/Documentation/networking/rxrpc.rst
> +++ b/Documentation/networking/rxrpc.rst
> @@ -878,8 +878,10 @@ The kernel interface functions are as follows:
> exclusively to in-kernel virtual addresses. msg.msg_flags may be given
> MSG_MORE if there will be subsequent data sends for this call.
>
> - The msg must not specify a destination address, control data or any flags
> - other than MSG_MORE.
> + msg must not specify a destination address, control data or any flags
> + other than MSG_MORE. The last-packet flag will only be set on the
> + outgoing packet if MSG_MORE is not set and all the data in the iterator is
> + buffered.
>
> notify_end_rx can be NULL or it can be used to specify a function to be
> called when the call changes state to end the Tx phase. This function is
[Severity: Low]
This isn't a bug introduced by this patch, but it is in the same
rxrpc_kernel_send_data() block that is being edited. The prototype
documented just above this hunk doesn't match the header:
Documentation/networking/rxrpc.rst:
typedef void (*rxrpc_notify_end_tx_t)(struct sock *sk,
unsigned long user_call_ID,
struct sk_buff *skb);
include/net/af_rxrpc.h:
typedef void (*rxrpc_notify_end_tx_t)(struct sock *, struct rxrpc_call *,
unsigned long);
rxrpc_notify_end_tx() in net/rxrpc/sendmsg.c calls it as
notify_end_tx(&rx->sk, call, call->user_call_ID), which matches the header.
So the documented version has the wrong argument types and order, and an
sk_buff argument that doesn't exist.
The documentation also calls the parameter notify_end_rx, but the header
calls it notify_end_tx.
Could the documented prototype and parameter name be updated to match
include/net/af_rxrpc.h?
[ ... ]
> diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
> index 80c6afc647870..e3b6347888bfe 100644
> --- a/net/rxrpc/sendmsg.c
> +++ b/net/rxrpc/sendmsg.c
[ ... ]
> @@ -372,6 +373,13 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
> ret = -EPROTO;
> goto out_unlock;
> }
> + if (unlikely(test_bit(RXRPC_CALL_TX_ERROR, &call->flags))) {
> + trace_rxrpc_abort(call->debug_id, rxrpc_sendmsg_tx_error,
> + call->cid, call->call_id, call->rx_consumed,
> + 0, -EIO);
> + ret = -EIO;
> + goto out_unlock;
> + }
[Severity: Low]
This changes what userspace sees from sendmsg() on AF_RXRPC sockets. If
->secure_packet() fails with anything other than -ENOMEM,
rxrpc_send_data() sets RXRPC_CALL_TX_ERROR. From then on, every data send
on that call returns -EIO here until the application aborts the call.
Should this be described in Documentation/networking/rxrpc.rst? The only
change there is to the rxrpc_kernel_send_data() paragraph.
The "Notes on sendmsg" section and the client/server walkthroughs don't
say that a call can become unusable for sending, that it then returns
-EIO, or that it has to be aborted explicitly. Right now the only place
this is described is item (5) of the comment in rxrpc_send_data().
The ENOMEM rewind can make a !MSG_MORE sendmsg() return a short count, but
the existing text about returning the amount of data consumed already
covers that. The missing part is mainly the sticky -EIO state and the need
to abort.
[ ... ]
> @@ -481,22 +505,44 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
> *
> * (4) If another sendmsg() has already queued the last packet: -EPROTO.
> *
> - * (5) If we queue the last packet: the amount copied (which may be
> + * (5) If an error occurs that may have corrupted the transmission
> + * buffer (e.g. crypto failure) or unusable crypto was encountered:
> + * the error given (and RXRPC_CALL_TX_ERROR is set to cause -EIO to
> + * be returned from further calls).
> + *
> + * (6) If we queue the last packet: the amount copied (which may be
> * zero). recvmsg() should be used to collect the result.
> *
> - * (6) If some data has been copied by this call: the amount copied
> + * (7) If some data has been copied by this call: the amount copied
> * (which will be greater than zero).
> *
> - * (7) Any other error.
> + * (8) Any other error.
> *
> - * For (1)-(4), there's no point in continuing with the sendmsg(). The
> - * app should abort the call (just in case the error came from
> - * somewhere else) and then use recvmsg() to collect the final result
> - * of the call.
> + * For (1)-(6), there's no point in continuing with the sendmsg() and
> + * we no longer care how much has been queued as the call is no longer
> + * viable. The app should abort the call (just in case the error came
> + * from somewhere else) and then use recvmsg() to collect the final
> + * result of the call.
[Severity: Low]
Should this range be (1)-(5)?
After the renumbering, item (6) is the case where the last packet was
queued. That is the normal, successful end of the Tx phase.
Saying the call is no longer viable and should be aborted contradicts
item (6) itself, which says recvmsg() should be used to collect the
result.
It also doesn't match rxrpc_kernel_send_data(), which treats consuming
all the data as success:
if (msg_data_left(msg) == 0) {
ret = 0;
break;
}
Documentation/networking/rxrpc.rst agrees, saying the function "returns 0
if all the data is queued".
None of the later patches in the series touch net/rxrpc/sendmsg.c, so the
wrong range would stay in the final state.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (8 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 09/17] rxrpc: Fix packet encryption error handling David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:36 ` [PATCH net v11 11/17] rxrpc: Fix double IRQ enablement David Howells
` (7 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman, stable
When rxrpc_recvmsg_data() gets called on a service call that has received
all of the request, RXRPC_CALL_RECVMSG_READ_ALL has been set, and this
causes rxrpc_recvmsg_data() to jump straight out, indicating the end of the
call (ie. rxrpc_kernel_recv_data() returns 1) without waiting for the call
to be processed or the reply to be transmitted.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
fs/afs/rxrpc.c | 4 ++--
net/rxrpc/recvmsg.c | 13 ++++++++++---
net/rxrpc/rxperf.c | 4 ++--
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index 0b9027549580..8e7a83a95ac9 100644
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -542,7 +542,7 @@ void afs_deliver_to_call(struct afs_call *call)
&call->service_id);
trace_afs_receive_data(call, &call->def_iter, false, ret);
- if (ret == -EINPROGRESS || ret == -EAGAIN)
+ if (ret == -EAGAIN || ret == 2)
return;
if (ret < 0 || ret == 1) {
if (ret == 1)
@@ -932,7 +932,7 @@ int afs_extract_data(struct afs_call *call, bool want_more)
return ret;
state = READ_ONCE(call->state);
- if (ret == 1) {
+ if (ret == 1 || ret == 2) {
switch (state) {
case AFS_CALL_CL_AWAIT_REPLY:
afs_set_call_state(call, state, AFS_CALL_CL_PROC_REPLY);
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index efcba4b2e74f..716f7f47d7e4 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -637,9 +637,11 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
* Note that we may return %-EAGAIN to drain empty packets at the end
* of the data, even if we've already copied over the requested data.
*
- * Return: %0 if got what was asked for and there's more available, %1
- * if we got what was asked for and we're at the end of the data and
- * %-EAGAIN if we need more data.
+ * Return: %0 if got what was asked for and there's more available, %1 if we
+ * got what was asked for and we're at the end of the call, %2 if a service
+ * call received all of the request but is still in progress and %-EAGAIN if we
+ * need more data. A variety of other errors can be returned if the call
+ * completed with failure.
*/
int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
struct iov_iter *iter, size_t *_len,
@@ -678,6 +680,11 @@ int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
read_phase_complete:
ret = 1;
+ if (rxrpc_is_service_call(call)) {
+ if (rxrpc_call_is_complete(call))
+ goto call_failed;
+ ret = 2;
+ }
out:
if (_service)
*_service = call->dest_srx.srx_service;
diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
index 5042e8bfca55..6ccfd40b5388 100644
--- a/net/rxrpc/rxperf.c
+++ b/net/rxrpc/rxperf.c
@@ -300,7 +300,7 @@ static void rxperf_deliver_to_call(struct work_struct *work)
&len, false, &remote_abort,
&call->service_id);
- if (ret == -EINPROGRESS || ret == -EAGAIN)
+ if (ret == -EAGAIN || ret == 2)
return;
if (ret < 0 || ret == 1) {
if (ret == 1)
@@ -379,7 +379,7 @@ static int rxperf_extract_data(struct rxperf_call *call, bool want_more)
if (ret == 0 || ret == -EAGAIN)
return ret;
- if (ret == 1) {
+ if (ret == 1 || ret == 2) {
switch (call->state) {
case RXPERF_CALL_SV_AWAIT_REQUEST:
rxperf_set_call_state(call, RXPERF_CALL_SV_REPLYING);
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls
2026-09-23 13:36 ` [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit is Cc'd to stable@vger.kernel.org but has no Fixes:
tag.
- [Low] The patch adds a new normal return value, 2, to the exported
kernel API rxrpc_kernel_recv_data(), but…
Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but rxperf can
leak rxperf_call objects, and the rxrpc_call references they hold,…
--
rxrpc: Fix return in rxrpc_recvmsg_data() for service calls
This patch makes rxrpc_kernel_recv_data() return a new value, 2, when a
service call has received all of its request but has not finished yet. It
no longer returns 1 in that case, which made the call look complete. The
AFS and rxperf callers are updated to handle the new return value.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914151340.3227501-1-dhowells%40redhat.com
> cc: linux-afs@lists.infradead.org
> cc: stable@vger.kernel.org
[Severity: Low]
Should this have a Fixes: tag? It is Cc'd to stable, but nothing says
which trees need it.
The behaviour being fixed looks like it came from:
Fixes: 93368b6bd58a ("rxrpc: Move call state changes from recvmsg to I/O thread")
That commit dropped the state switch in rxrpc_kernel_recv_data(). The
switch had a default case that returned -EINPROGRESS for service calls
that were still processing, replying or waiting for the final ACK. In its
place, it added the RXRPC_CALL_RECVMSG_READ_ALL early return in
rxrpc_recvmsg_data(), which returns 1:
if (test_bit(RXRPC_CALL_RECVMSG_READ_ALL, &call->flags)) {
seq = call->ackr_window - 1;
ret = 1;
goto done;
}
Since then, the ret == -EINPROGRESS checks in afs_deliver_to_call() and
rxperf_deliver_to_call() have been dead code.
Trees older than v6.2 still use the -EINPROGRESS contract. Without the
tag, stable maintainers could backport this to trees that don't need it.
> diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
> index efcba4b2e74f0..716f7f47d7e46 100644
> --- a/net/rxrpc/recvmsg.c
> +++ b/net/rxrpc/recvmsg.c
> @@ -637,9 +637,11 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> * Note that we may return %-EAGAIN to drain empty packets at the end
> * of the data, even if we've already copied over the requested data.
> *
> - * Return: %0 if got what was asked for and there's more available, %1
> - * if we got what was asked for and we're at the end of the data and
> - * %-EAGAIN if we need more data.
> + * Return: %0 if got what was asked for and there's more available, %1 if we
> + * got what was asked for and we're at the end of the call, %2 if a service
> + * call received all of the request but is still in progress and %-EAGAIN if we
> + * need more data. A variety of other errors can be returned if the call
> + * completed with failure.
> */
> int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
> struct iov_iter *iter, size_t *_len,
> @@ -678,6 +680,11 @@ int rxrpc_kernel_recv_data(struct socket *sock, struct rxrpc_call *call,
>
> read_phase_complete:
> ret = 1;
> + if (rxrpc_is_service_call(call)) {
> + if (rxrpc_call_is_complete(call))
> + goto call_failed;
> + ret = 2;
> + }
[Severity: Low]
Should Documentation/networking/rxrpc.rst be updated as well? Its section
on rxrpc_kernel_recv_data() still lists only three normal returns:
There are three normal returns: 0 if the buffer was filled and want_more
was true; 1 if the buffer was filled, the last DATA packet has been
emptied and want_more was false; and -EAGAIN if the function needs to be
called again.
With this change, a service call in that situation gets 2, or goes to
call_failed if the call has already completed. It no longer gets 1.
The kernel-doc above now lists %2, so the tree describes two different
return contracts for this exported function. A kernel service written from
rxrpc.rst would not know to handle 2.
> out:
> if (_service)
> *_service = call->dest_srx.srx_service;
> diff --git a/net/rxrpc/rxperf.c b/net/rxrpc/rxperf.c
> index 5042e8bfca550..6ccfd40b5388c 100644
> --- a/net/rxrpc/rxperf.c
> +++ b/net/rxrpc/rxperf.c
> @@ -300,7 +300,7 @@ static void rxperf_deliver_to_call(struct work_struct *work)
> &len, false, &remote_abort,
> &call->service_id);
>
> - if (ret == -EINPROGRESS || ret == -EAGAIN)
> + if (ret == -EAGAIN || ret == 2)
> return;
[Severity: Medium]
This is a pre-existing issue that this patch did not introduce, but can
rxperf leak rxperf_call objects, and the rxrpc_call refs they hold, when
the module is unloaded?
The only place an rxperf_call is freed is the call_complete path in
rxperf_deliver_to_call(). That path runs only when a notify_rx callback
queues the work item.
The teardown in rxperf_close_socket() looks like this:
kernel_listen(rxperf_socket, 0);
kernel_sock_shutdown(rxperf_socket, SHUT_RDWR);
flush_workqueue(rxperf_workqueue);
sock_release(rxperf_socket);
kernel_sock_shutdown() goes into rxrpc_shutdown(), which sets sk_state to
RXRPC_CLOSE. After that, sock_release()->rxrpc_release_calls_on_socket()
aborts any remaining calls with rxrpc_propose_abort(). The completion
notification is then dropped by this check in rxrpc_notify_socket():
if (rx && sk->sk_state < RXRPC_CLOSE) {
if (call->notify_rx) {
rxperf does not keep a list of outstanding calls. So a call still in
flight at unload, such as a partly received request or a call in
SV_AWAIT_ACK, never gets its rxperf_call freed or its rxcall ref dropped.
This patch returns early on ret == 2 in SV_AWAIT_ACK. Such calls now
reliably stay alive until the final ACK or completion arrives, so the
window for this is somewhat longer. The teardown gap itself is older than
this patch.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 11/17] rxrpc: Fix double IRQ enablement
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (9 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:36 ` [PATCH net v11 12/17] rxrpc: Fix generation of notifications after call completion David Howells
` (6 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
rxrpc_notify_socket() explicitly disables and then re-enables IRQs, but one
of its call chains (rxrpc_input_queue_data() -> rxrpc_end_rx_phase() ->
rxrpc_call_completed() -> rxrpc_set_call_completion()) has IRQs disabled
around it.
Fix this by making rxrpc_notify_socket() use irqsave spinlocks.
Fixes: a2ea9a907260 ("rxrpc: Use irq-disabling spinlocks between app and I/O thread")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260812110129.979970-6-dhowells@redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@kernel.org
---
net/rxrpc/recvmsg.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 716f7f47d7e4..0c960f13b5fc 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -24,6 +24,7 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
{
struct rxrpc_sock *rx;
struct sock *sk;
+ unsigned long flags;
_enter("%d", call->debug_id);
@@ -38,16 +39,16 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
sk = &rx->sk;
if (rx && sk->sk_state < RXRPC_CLOSE) {
if (call->notify_rx) {
- spin_lock_irq(&call->notify_lock);
+ spin_lock_irqsave(&call->notify_lock, flags);
call->notify_rx(sk, call, call->user_call_ID);
- spin_unlock_irq(&call->notify_lock);
+ spin_unlock_irqrestore(&call->notify_lock, flags);
} else {
- spin_lock_irq(&rx->recvmsg_lock);
+ spin_lock_irqsave(&rx->recvmsg_lock, flags);
if (list_empty(&call->recvmsg_link)) {
rxrpc_get_call(call, rxrpc_call_get_notify_socket);
list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
}
- spin_unlock_irq(&rx->recvmsg_lock);
+ spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
if (!sock_flag(sk, SOCK_DEAD)) {
_debug("call %ps", sk->sk_data_ready);
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 12/17] rxrpc: Fix generation of notifications after call completion
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (10 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 11/17] rxrpc: Fix double IRQ enablement David Howells
@ 2026-09-23 13:36 ` David Howells
2026-09-23 13:37 ` [PATCH net v11 13/17] rxrpc: Fix RxGK key parser to check enctype is supported David Howells
` (5 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:36 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
AF_RXRPC may generate a notification to the application after a call has
completed because it generates one notification when
rxrpc_input_split_jumbo() queues the final packet and completes the call
and then generates another when rxrpc_input_split_jumbo() does the
aggregated data receive notification at the end of the function.
This might cause the AFS filesystem to malfunction because it tries to
queue the afs_call for processing an extra time. Most of the time this
happens quickly enough that the second queue_work skips, but sometimes this
means that the call work may happen a second time with implications for
afs_call lifetime management.
Fix this by:
(1) Create a lighter version of rxrpc_notify_socket() that's just used to
requeue a call for rxrpc_recvmsg() without needing to consider kernel
apps. This also ignores shutdown(), allowing recvmsg() to continue
collecting from already queued calls.
(2) Move rxrpc_notify_socket() to call_state.c and rename it to
__rxrpc_notify_socket().
(3) Create a wrapper called rxrpc_notify_socket() that skips the
notification if a call is completed.
(4) Make rxrpc_set_call_completion() call __rxrpc_notify_socket() to avoid
the skip-if-completed check.
Also remove the comment on rxrpc_notify_socket() that said it added the
call to a dummy queue to prevent further notification.
Fixes: 2d1faf7a0ca3 ("rxrpc: Simplify skbuff accounting in receive path")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@kernel.org
---
include/trace/events/rxrpc.h | 1 +
net/rxrpc/ar-internal.h | 2 +-
net/rxrpc/call_state.c | 57 +++++++++++++++++++++++++++++++++++-
net/rxrpc/recvmsg.c | 45 ++++++++++------------------
4 files changed, 74 insertions(+), 31 deletions(-)
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index a5c92592d8f9..52f8718cf725 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -343,6 +343,7 @@
EM(rxrpc_call_see_distribute_error, "SEE dist-err") \
EM(rxrpc_call_see_input, "SEE input ") \
EM(rxrpc_call_see_notify_released, "SEE nfy-rlsd") \
+ EM(rxrpc_call_see_notify_skipped, "SEE nfy-skip") \
EM(rxrpc_call_see_recvmsg, "SEE recvmsg ") \
EM(rxrpc_call_see_recvmsg_requeue, "SEE recv-rqu") \
EM(rxrpc_call_see_recvmsg_requeue_first, "SEE recv-rqF") \
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index a6f830c1621f..cb36a709f540 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -1110,6 +1110,7 @@ static inline bool rxrpc_is_client_call(const struct rxrpc_call *call)
/*
* call_state.c
*/
+void rxrpc_notify_socket(struct rxrpc_call *call);
bool rxrpc_set_call_completion(struct rxrpc_call *call,
enum rxrpc_call_completion compl,
u32 abort_code,
@@ -1442,7 +1443,6 @@ extern const struct seq_operations rxrpc_local_seq_ops;
/*
* recvmsg.c
*/
-void rxrpc_notify_socket(struct rxrpc_call *);
int rxrpc_recvmsg(struct socket *, struct msghdr *, size_t, int);
/*
diff --git a/net/rxrpc/call_state.c b/net/rxrpc/call_state.c
index 6afb54373ebb..52465e88a044 100644
--- a/net/rxrpc/call_state.c
+++ b/net/rxrpc/call_state.c
@@ -7,6 +7,61 @@
#include "ar-internal.h"
+/*
+ * Post a call for attention by the socket or kernel service.
+ */
+static void __rxrpc_notify_socket(struct rxrpc_call *call)
+{
+ struct rxrpc_sock *rx;
+ struct sock *sk;
+ unsigned long flags;
+
+ if (test_bit(RXRPC_CALL_RELEASED, &call->flags)) {
+ rxrpc_see_call(call, rxrpc_call_see_notify_released);
+ return;
+ }
+
+ rcu_read_lock();
+
+ rx = rcu_dereference(call->socket);
+ sk = &rx->sk;
+ if (rx && sk->sk_state < RXRPC_CLOSE) {
+ if (call->notify_rx) {
+ spin_lock_irqsave(&call->notify_lock, flags);
+ call->notify_rx(sk, call, call->user_call_ID);
+ spin_unlock_irqrestore(&call->notify_lock, flags);
+ } else {
+ spin_lock_irqsave(&rx->recvmsg_lock, flags);
+ if (list_empty(&call->recvmsg_link)) {
+ rxrpc_get_call(call, rxrpc_call_get_notify_socket);
+ list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
+ }
+ spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
+
+ if (!sock_flag(sk, SOCK_DEAD)) {
+ _debug("call %ps", sk->sk_data_ready);
+ sk->sk_data_ready(sk);
+ }
+ }
+ }
+
+ rcu_read_unlock();
+}
+
+/*
+ * Post a call for attention by the socket or kernel service if the call isn't
+ * already complete.
+ */
+void rxrpc_notify_socket(struct rxrpc_call *call)
+{
+ if (rxrpc_call_is_complete(call)) {
+ rxrpc_see_call(call, rxrpc_call_see_notify_skipped);
+ return;
+ }
+
+ __rxrpc_notify_socket(call);
+}
+
/*
* Transition a call to the complete state.
*/
@@ -25,7 +80,7 @@ bool rxrpc_set_call_completion(struct rxrpc_call *call,
rxrpc_set_call_state(call, RXRPC_CALL_COMPLETE);
trace_rxrpc_call_complete(call);
wake_up(&call->waitq);
- rxrpc_notify_socket(call);
+ __rxrpc_notify_socket(call);
return true;
}
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 0c960f13b5fc..8b07c31dfd2e 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -17,14 +17,14 @@
#include "ar-internal.h"
/*
- * Post a call for attention by the socket or kernel service. Further
- * notifications are suppressed by putting recvmsg_link on a dummy queue.
+ * Requeue a call for recvmsg() to pick up. We ignore RXRPC_CLOSE, allowing
+ * recvmsg() to continue picking up calls that are already on the queue if it
+ * wants to, but no new calls will get added.
*/
-void rxrpc_notify_socket(struct rxrpc_call *call)
+static void rxrpc_requeue_call(struct socket *sock, struct rxrpc_call *call)
{
- struct rxrpc_sock *rx;
- struct sock *sk;
- unsigned long flags;
+ struct rxrpc_sock *rx = rxrpc_sk(sock->sk);
+ struct sock *sk = &rx->sk;
_enter("%d", call->debug_id);
@@ -33,31 +33,18 @@ void rxrpc_notify_socket(struct rxrpc_call *call)
return;
}
- rcu_read_lock();
-
- rx = rcu_dereference(call->socket);
- sk = &rx->sk;
- if (rx && sk->sk_state < RXRPC_CLOSE) {
- if (call->notify_rx) {
- spin_lock_irqsave(&call->notify_lock, flags);
- call->notify_rx(sk, call, call->user_call_ID);
- spin_unlock_irqrestore(&call->notify_lock, flags);
- } else {
- spin_lock_irqsave(&rx->recvmsg_lock, flags);
- if (list_empty(&call->recvmsg_link)) {
- rxrpc_get_call(call, rxrpc_call_get_notify_socket);
- list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
- }
- spin_unlock_irqrestore(&rx->recvmsg_lock, flags);
+ spin_lock_irq(&rx->recvmsg_lock);
+ if (list_empty(&call->recvmsg_link)) {
+ rxrpc_get_call(call, rxrpc_call_get_notify_socket);
+ list_add_tail(&call->recvmsg_link, &rx->recvmsg_q);
+ }
+ spin_unlock_irq(&rx->recvmsg_lock);
- if (!sock_flag(sk, SOCK_DEAD)) {
- _debug("call %ps", sk->sk_data_ready);
- sk->sk_data_ready(sk);
- }
- }
+ if (!sock_flag(sk, SOCK_DEAD)) {
+ _debug("call %ps", sk->sk_data_ready);
+ sk->sk_data_ready(sk);
}
- rcu_read_unlock();
_leave("");
}
@@ -562,7 +549,7 @@ int rxrpc_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
if (!(flags & MSG_PEEK) &&
!skb_queue_empty(&call->recvmsg_queue))
- rxrpc_notify_socket(call);
+ rxrpc_requeue_call(sock, call);
goto not_yet_complete;
call_failed:
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 13/17] rxrpc: Fix RxGK key parser to check enctype is supported
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (11 preceding siblings ...)
2026-09-23 13:36 ` [PATCH net v11 12/17] rxrpc: Fix generation of notifications after call completion David Howells
@ 2026-09-23 13:37 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:37 ` [PATCH net v11 14/17] afs: Fix creation of RxGK CM channel token to have right size David Howells
` (4 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:37 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
Fix the parser of RxGK keys supplied by userspace to check that the
specified encryption type is supported and check the key length. Further,
since the checking function isn't necessarily available in CONFIG_RXGK=n,
make the RxGK key wrangling bits conditional.
Also only account the a token to the key's quota if that token is used.
Fixes: 0ca100ff4df6 ("rxrpc: Add YFS RxGK (GSSAPI) security class")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
net/rxrpc/key.c | 41 +++++++++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index cbd26da44951..904da3fe7e47 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -71,14 +71,11 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
if (toklen < 8 * 4 + tktlen)
return -EKEYREJECTED;
- plen = sizeof(*token) + sizeof(*token->kad) + tktlen;
- prep->quotalen += datalen + plen;
-
- plen -= sizeof(*token);
token = kzalloc_obj(*token);
if (!token)
return -ENOMEM;
+ plen = sizeof(*token->kad) + tktlen;
token->kad = kzalloc(plen, GFP_KERNEL);
if (!token->kad) {
kfree(token);
@@ -112,6 +109,8 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
token->kad->ticket[4], token->kad->ticket[5],
token->kad->ticket[6], token->kad->ticket[7]);
+ prep->quotalen += sizeof(*token) + datalen + plen;
+
/* count the number of tokens attached */
prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
@@ -129,6 +128,7 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
return 0;
}
+#ifdef CONFIG_RXGK
static u64 xdr_dec64(const __be32 *xdr)
{
return (u64)ntohl(xdr[0]) << 32 | (u64)ntohl(xdr[1]);
@@ -166,12 +166,13 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
size_t datalen,
const __be32 *xdr, unsigned int toklen)
{
+ const struct krb5_enctype *enc;
struct rxrpc_key_token *token, **pptoken;
time64_t expiry;
- size_t plen;
const __be32 *ticket, *key;
s64 tmp;
size_t raw_keylen, raw_tktlen, keylen, tktlen;
+ int ret = -EKEYREJECTED;
_enter(",{%x,%x,%x,%x},%x",
ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
@@ -202,10 +203,6 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
goto reject;
}
- plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
- prep->quotalen += datalen + plen;
-
- plen -= sizeof(*token);
token = kzalloc_obj(*token);
if (!token)
goto nomem;
@@ -229,6 +226,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
token->rxgk->key.data = token->rxgk->_key;
token->rxgk->ticket.len = raw_tktlen;
+ /* Check the enctype is supported. */
+ enc = crypto_krb5_find_enctype(token->rxgk->enctype);
+ if (!enc) {
+ ret = -ENOPKG;
+ goto reject_token;
+ }
+ if (raw_keylen != enc->key_len) {
+ ret = -EKEYREJECTED;
+ goto reject_token;
+ }
+
if (token->rxgk->endtime != 0) {
expiry = rxrpc_s64_to_time64(token->rxgk->endtime);
if (expiry < 0)
@@ -257,6 +265,8 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
_debug("TICK: %*phN",
min_t(u32, token->rxgk->ticket.len, 32), token->rxgk->ticket.data);
+ prep->quotalen += sizeof(*token) + datalen + tktlen + keylen;
+
/* count the number of tokens attached */
prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
@@ -280,12 +290,13 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
kfree(token->rxgk);
kfree(token);
reject:
- return -EKEYREJECTED;
+ return ret;
expired:
kfree(token->rxgk);
kfree(token);
return -EKEYEXPIRED;
}
+#endif /* CONFIG_RXGK */
/*
* attempt to parse the data as the XDR format
@@ -386,9 +397,11 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
case RXRPC_SECURITY_RXKAD:
ret2 = rxrpc_preparse_xdr_rxkad(prep, datalen, token, toklen);
break;
+#ifdef CONFIG_RXGK
case RXRPC_SECURITY_YFS_RXGK:
ret2 = rxrpc_preparse_xdr_yfs_rxgk(prep, datalen, token, toklen);
break;
+#endif
default:
ret2 = -EPROTONOSUPPORT;
break;
@@ -556,10 +569,12 @@ static void rxrpc_free_token_list(struct rxrpc_key_token *token)
case RXRPC_SECURITY_RXKAD:
kfree(token->kad);
break;
+#ifdef CONFIG_RXGK
case RXRPC_SECURITY_YFS_RXGK:
kfree(token->rxgk->ticket.data);
kfree(token->rxgk);
break;
+#endif
default:
pr_err("Unknown token type %x on rxrpc key\n",
token->security_index);
@@ -603,9 +618,11 @@ static void rxrpc_describe(const struct key *key, struct seq_file *m)
case RXRPC_SECURITY_RXKAD:
seq_puts(m, "ka");
break;
+#ifdef CONFIG_RXGK
case RXRPC_SECURITY_YFS_RXGK:
seq_puts(m, "ygk");
break;
+#endif
default: /* we have a ticket we can't encode */
seq_printf(m, "%u", token->security_index);
break;
@@ -770,12 +787,14 @@ static long rxrpc_read(const struct key *key,
toksize += RND(token->kad->ticket_len);
break;
+#ifdef CONFIG_RXGK
case RXRPC_SECURITY_YFS_RXGK:
toksize += 6 * 8 + 2 * 4;
if (!token->no_leak_key)
toksize += RND(token->rxgk->key.len);
toksize += RND(token->rxgk->ticket.len);
break;
+#endif
default: /* we have a ticket we can't encode */
pr_err("Unsupported key token type (%u)\n",
@@ -856,6 +875,7 @@ static long rxrpc_read(const struct key *key,
ENCODE_DATA(token->kad->ticket_len, token->kad->ticket);
break;
+#ifdef CONFIG_RXGK
case RXRPC_SECURITY_YFS_RXGK:
ENCODE64(token->rxgk->begintime);
ENCODE64(token->rxgk->endtime);
@@ -869,6 +889,7 @@ static long rxrpc_read(const struct key *key,
ENCODE_DATA(token->rxgk->key.len, token->rxgk->key.data);
ENCODE_DATA(token->rxgk->ticket.len, token->rxgk->ticket.data);
break;
+#endif
default:
pr_err("Unsupported key token type (%u)\n",
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 13/17] rxrpc: Fix RxGK key parser to check enctype is supported
2026-09-23 13:37 ` [PATCH net v11 13/17] rxrpc: Fix RxGK key parser to check enctype is supported David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 2 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The patch rewrote the RxGK key quota charge in
rxrpc_preparse_xdr_yfs_rxgk() (net/rxrpc/key.c) and dropped the size…
- [Medium] The new key-length check in rxrpc_preparse_xdr_yfs_rxgk()
(net/rxrpc/key.c:235-238, `if (raw_keylen != enc->key_len) { ret =…
--
rxrpc: Fix RxGK key parser to check enctype is supported
The RxGK key parser now checks that the token's enctype is supported and
that the key length matches that enctype. The RxGK key handling is now
built only when CONFIG_RXGK is set. Quota is now charged in both the rxkad
and RxGK parsers only after a token is accepted.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
> diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
> index cbd26da449510..904da3fe7e478 100644
> --- a/net/rxrpc/key.c
> +++ b/net/rxrpc/key.c
[ ... ]
> @@ -202,10 +203,6 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
> goto reject;
> }
>
> - plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
> - prep->quotalen += datalen + plen;
> -
> - plen -= sizeof(*token);
> token = kzalloc_obj(*token);
> if (!token)
> goto nomem;
> @@ -229,6 +226,17 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
> token->rxgk->key.data = token->rxgk->_key;
> token->rxgk->ticket.len = raw_tktlen;
>
> + /* Check the enctype is supported. */
> + enc = crypto_krb5_find_enctype(token->rxgk->enctype);
> + if (!enc) {
> + ret = -ENOPKG;
> + goto reject_token;
> + }
> + if (raw_keylen != enc->key_len) {
> + ret = -EKEYREJECTED;
> + goto reject_token;
> + }
[Severity: Medium]
Does the server side handle this new -EKEYREJECTED return? The server
reaches this parser through this chain:
rxgk_verify_response()
-> rxgk_extract_token()
-> rxgk_yfs_decode_ticket()
-> key_instantiate_and_link()
-> rxrpc_preparse()
-> rxrpc_preparse_xdr()
-> rxrpc_preparse_xdr_yfs_rxgk()
rxgk_yfs_decode_ticket() compares klen with the ticket length but not with
the enctype. It returns the error from key_instantiate_and_link() unchanged.
The cant_get_token switch in rxgk_extract_token() has no case for
-EKEYREJECTED:
net/rxrpc/rxgk_app.c:rxgk_extract_token() {
...
cant_get_token:
switch (ret) {
case -ENOMEM:
goto temporary_error;
case -EINVAL:
return rxrpc_abort_conn(conn, skb, RXGK_NOTAUTH, -EKEYREJECTED,
rxgk_abort_resp_tok_internal_error);
case -ENOPKG:
return rxrpc_abort_conn(conn, skb, KRB5_PROG_KEYTYPE_NOSUPP,
-EKEYREJECTED, rxgk_abort_resp_tok_nopkg);
}
temporary_error:
...
return ret;
}
So the error is handled like a temporary error. rxgk_verify_response()
then does goto out, the RESPONSE is dropped, and the connection stays in
RXRPC_CONN_SERVICE_CHALLENGING.
Before this patch, the same ticket got through key instantiation and failed
later, in rxgk_generate_transport_key() -> rxgk_set_up_ciphers() ->
crypto_krb5_calc_PRFplus().
For the RFC3961 enctypes, a key length mismatch made rfc3961_calc_DK()
return -EINVAL, and the connection was aborted with RXGK_NOTAUTH. For the
other enctypes, the authenticator failed to decrypt and the connection was
aborted with RXGK_SEALEDINCON.
The ticket is decrypted with the server key, so only a buggy or
misconfigured token issuer can produce one like this.
Should the cant_get_token switch in rxgk_extract_token() get an
-EKEYREJECTED case? Then the client would get an abort instead of being
re-challenged until it times out.
> +
> if (token->rxgk->endtime != 0) {
> expiry = rxrpc_s64_to_time64(token->rxgk->endtime);
> if (expiry < 0)
[ ... ]
> @@ -257,6 +265,8 @@ static int rxrpc_preparse_xdr_yfs_rxgk(struct key_preparsed_payload *prep,
> _debug("TICK: %*phN",
> min_t(u32, token->rxgk->ticket.len, 32), token->rxgk->ticket.data);
>
> + prep->quotalen += sizeof(*token) + datalen + tktlen + keylen;
[Severity: Medium]
Was sizeof(*token->rxgk) meant to be dropped from the quota charge in
rxrpc_preparse_xdr_yfs_rxgk()? The removed code included it:
plen = sizeof(*token) + sizeof(*token->rxgk) + tktlen + keylen;
token->rxgk is still allocated separately, and that allocation includes the
fixed struct rxgk_key header:
token->rxgk = kzalloc_flex(struct rxgk_key, _key, raw_keylen);
The rxkad hunk in this same patch still includes sizeof(*token->kad) in
plen, so the two parsers now charge differently.
The datalen term doesn't make up the difference. rxrpc_preparse_xdr()
requires the remaining datalen to be 0 before it calls the token parsers,
so datalen is always 0 here.
So each accepted RxGK token uses about 72 bytes more (on 64-bit) than
generic_key_instantiate() reserves against the user's key quota.
Should this be something like:
prep->quotalen += sizeof(*token) + sizeof(*token->rxgk) + datalen +
tktlen + keylen;
> +
> /* count the number of tokens attached */
> prep->payload.data[1] = (void *)((unsigned long)prep->payload.data[1] + 1);
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 14/17] afs: Fix creation of RxGK CM channel token to have right size
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (12 preceding siblings ...)
2026-09-23 13:37 ` [PATCH net v11 13/17] rxrpc: Fix RxGK key parser to check enctype is supported David Howells
@ 2026-09-23 13:37 ` David Howells
2026-09-23 13:37 ` [PATCH net v11 15/17] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus David Howells
` (3 subsequent siblings)
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:37 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
stable
Fix afs_create_yfs_cm_token() so that it calculates the token size
correctly, remembering to add in the 4 bytes of the level.
Fixes: d98c317fd9aa ("afs: Use rxgk RESPONSE to pass token for callback channel")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
fs/afs/cm_security.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/afs/cm_security.c b/fs/afs/cm_security.c
index 103168c70dd4..5eeeef761cf3 100644
--- a/fs/afs/cm_security.c
+++ b/fs/afs/cm_security.c
@@ -235,7 +235,7 @@ static int afs_create_yfs_cm_token(struct sk_buff *challenge,
* struct RXGK_AuthName identities<>;
* };
*/
- toksize = keysize + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
+ toksize = keysize + 4 + 8 + 4 + 4 + 8 + xdr_len_object(authsize);
offset = 0;
encsize = crypto_krb5_how_much_buffer(token_krb5, KRB5_ENCRYPT_MODE, toksize, &offset);
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 15/17] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (13 preceding siblings ...)
2026-09-23 13:37 ` [PATCH net v11 14/17] afs: Fix creation of RxGK CM channel token to have right size David Howells
@ 2026-09-23 13:37 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-23 13:37 ` [PATCH net v11 16/17] afs: Fix uncleared op->call pointer David Howells
` (2 subsequent siblings)
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:37 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman, Jarkko Sakkinen, keyrings, stable
Fix afs_make_op_call to set call->server. This will prevent
afs_done_fs_inline_bulk_status() from oopsing if the server does not
support the FS.InlineBulkStatus RPC due to call->server not being set.
Note that this requires afs_make_op_call() to be moved so that it can use
afs_use_server(); the server is un-used by afs_free_call().
Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Jeffrey Altman <jaltman@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: Jarkko Sakkinen <jarkko@kernel.org>
cc: linux-afs@lists.infradead.org
cc: keyrings@vger.kernel.org
cc: stable@kernel.org
---
fs/afs/internal.h | 33 +++++++++++++++++----------------
include/trace/events/afs.h | 1 +
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 330654ed16ec..40dc85d9a5c9 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1424,22 +1424,6 @@ static inline void afs_see_call(struct afs_call *call, enum afs_call_trace why)
__builtin_return_address(0));
}
-static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
- gfp_t gfp)
-{
- struct afs_addr_list *alist = op->estate->addresses;
-
- op->call = afs_get_call(call, afs_call_trace_get);
- op->type = call->type;
- call->op = op;
- call->key = op->key;
- call->intr = !(op->flags & AFS_OPERATION_UNINTR);
- call->peer = rxrpc_kernel_get_peer(alist->addrs[op->addr_index].peer);
- call->service_id = op->server->service_id;
- afs_make_call(call, gfp);
- afs_put_call(call);
-}
-
static inline void afs_extract_begin(struct afs_call *call, void *buf, size_t size)
{
call->iov_len = size;
@@ -1771,6 +1755,23 @@ static inline struct inode *AFS_VNODE_TO_I(struct afs_vnode *vnode)
return &vnode->netfs.inode;
}
+static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
+ gfp_t gfp)
+{
+ struct afs_addr_list *alist = op->estate->addresses;
+
+ op->call = afs_get_call(call, afs_call_trace_get);
+ op->type = call->type;
+ call->op = op;
+ call->server = afs_use_server(op->server, false, afs_server_trace_use_call);
+ call->key = op->key;
+ call->intr = !(op->flags & AFS_OPERATION_UNINTR);
+ call->peer = rxrpc_kernel_get_peer(alist->addrs[op->addr_index].peer);
+ call->service_id = op->server->service_id;
+ afs_make_call(call, gfp);
+ afs_put_call(call);
+}
+
/*
* Note that a dentry got changed. We need to set d_fsdata to the data version
* number derived from the result of the operation. It doesn't matter if
diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index 1b3c48b5591d..04b0bb682b81 100644
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -148,6 +148,7 @@ enum yfs_cm_operation {
EM(afs_server_trace_unuse_slist_isort, "UNU isort") \
EM(afs_server_trace_update, "UPDATE ") \
EM(afs_server_trace_use_by_uuid, "USE uuid ") \
+ EM(afs_server_trace_use_call, "USE call ") \
EM(afs_server_trace_use_cm_call, "USE cm-cl") \
EM(afs_server_trace_use_get_caps, "USE gcaps") \
EM(afs_server_trace_use_give_up_cb, "USE gvupc") \
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 15/17] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus
2026-09-23 13:37 ` [PATCH net v11 15/17] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] This patch adds `call->server = afs_use_server(op->server,
false, afs_server_trace_use_call)` in afs_make_op_call()…
--
afs: Fix lack of setting call->server when doing FS.InlineBulkStatus
This commit makes afs_make_op_call() take an active count on op->server
and store it in call->server. That stops afs_done_fs_inline_bulk_status()
from oopsing when the server doesn't support FS.InlineBulkStatus. The
function moves further down internal.h so it can use afs_use_server(), and
a new afs_server_trace_use_call trace value is added.
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824091645.415423-1-dhowells%40redhat.com
> diff --git a/fs/afs/internal.h b/fs/afs/internal.h
> index 330654ed16ece..40dc85d9a5c95 100644
> --- a/fs/afs/internal.h
> +++ b/fs/afs/internal.h
[ ... ]
> @@ -1771,6 +1755,23 @@ static inline struct inode *AFS_VNODE_TO_I(struct afs_vnode *vnode)
> return &vnode->netfs.inode;
> }
>
> +static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
> + gfp_t gfp)
> +{
> + struct afs_addr_list *alist = op->estate->addresses;
> +
> + op->call = afs_get_call(call, afs_call_trace_get);
> + op->type = call->type;
> + call->op = op;
> + call->server = afs_use_server(op->server, false, afs_server_trace_use_call);
[Severity: Medium]
Can this leave a server record that is no longer used with no GC timer
armed?
With this change, every op call holds an active count on op->server.
afs_free_call() drops it with the _notime variant:
fs/afs/rxrpc.c:afs_free_call() {
...
afs_unuse_server_notime(call->net, call->server, afs_server_trace_unuse_call);
...
}
When active reaches zero on a server that hasn't expired, in a cell that is
still live, afs_unuse_server_notime() does nothing:
fs/afs/server.c:afs_unuse_server_notime() {
...
if (atomic_dec_and_test(&server->active)) {
if (test_bit(AFS_SERVER_FL_EXPIRED, &server->flags) ||
READ_ONCE(server->cell->state) >= AFS_CELL_REMOVING)
schedule_work(&server->destroyer);
}
...
}
Only afs_unuse_server() sets unuse_time and calls afs_set_server_timer()
when active reaches zero. Before this patch, op calls didn't pin the
server, so the last active count was always dropped by the server list's
afs_unuse_server().
In the async read path, the call can outlive the op's server list:
afs_fetch_data_async_rx()
afs_read_receive()
afs_put_call(call) <- drops the op's ref
afs_select_fileserver(op) <- may afs_put_serverlist() the old list
or
afs_end_read()->afs_put_operation()->afs_put_serverlist()
afs_put_call(call) <- work item's ref, reaches afs_free_call()
Suppose the volume's server list is replaced while the read is in flight,
for example after a VLDB update or a VMOVED abort, and server X is in no
other list. The old list's afs_unuse_server() takes active from 2 to 1,
which doesn't arm the timer. The call's afs_unuse_server_notime() then
takes it from 1 to 0, which does nothing.
Would server X then stay in cell->fs_servers and on the probe lists? It
would keep being probed and its callbacks would not be given up, until
afs_purge_servers() runs at cell removal or the server is looked up again.
The periodic afs_fs_get_capabilities() probes also unuse with the _notime
variant, so they wouldn't re-arm GC either.
Probe and CM calls already have the same _notime behaviour. This change
extends it to every FS operation call, including async calls that often
outlive their op.
Could op calls be released with afs_unuse_server() instead? Another option
is for afs_unuse_server_notime() to set unuse_time and arm the timer when
active reaches zero on a live server.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH net v11 16/17] afs: Fix uncleared op->call pointer
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (14 preceding siblings ...)
2026-09-23 13:37 ` [PATCH net v11 15/17] afs: Fix lack of setting call->server when doing FS.InlineBulkStatus David Howells
@ 2026-09-23 13:37 ` David Howells
2026-09-23 13:37 ` [PATCH net v11 17/17] rxrpc: fix use-after-free in rxrpc_poke_conn() David Howells
2026-09-24 8:45 ` [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls David Howells
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-23 13:37 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel
Fix uncleared op->call pointer in afs_wait_for_operation() to make sure
that it isn't seen set if server rotation occurs. Note that
afs_read_receive() already does this. Note also that this was spotted by
AI code inspection rather than by hitting an issue.
Fixes: e49c7b2f6de7 ("afs: Build an abstraction around an "operation" concept")
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907113743.1453210-1-dhowells%40redhat.com
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
---
fs/afs/fs_operation.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/afs/fs_operation.c b/fs/afs/fs_operation.c
index 20801b29521d..94fa65548d71 100644
--- a/fs/afs/fs_operation.c
+++ b/fs/afs/fs_operation.c
@@ -297,6 +297,7 @@ void afs_wait_for_operation(struct afs_operation *op)
op->call_error = op->call->error;
op->call_responded = op->call->responded;
afs_put_call(op->call);
+ op->call = NULL;
}
}
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH net v11 17/17] rxrpc: fix use-after-free in rxrpc_poke_conn()
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (15 preceding siblings ...)
2026-09-23 13:37 ` [PATCH net v11 16/17] afs: Fix uncleared op->call pointer David Howells
@ 2026-09-23 13:37 ` David Howells
2026-09-27 14:59 ` netdev-bot+sashiko
2026-09-24 8:45 ` [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls David Howells
17 siblings, 1 reply; 27+ messages in thread
From: David Howells @ 2026-09-23 13:37 UTC (permalink / raw)
To: netdev
Cc: David Howells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Seungwon Bae, stable
From: Seungwon Bae <qotmddnjs@ajou.ac.kr>
rxrpc_poke_conn() takes a reference on the connection with no liveness
check, unlike its sibling rxrpc_queue_conn() which gates on
atomic_read(&conn->active) >= 0. The per-connection timer is armed with
no reference held for it, and rxrpc_put_connection() cancels it with a
non-synchronous timer_delete() only after the refcount reaches 0.
refcount_t saturates rather than resurrecting, so the connection can be
kfree()d while still linked in local->conn_attend_q (nothing in teardown
unlinks attend_link). The rxrpc I/O thread then performs a UAF write
(list_del_init) plus UAF reads and indirect calls through conn->security.
Reproduced on a KASAN + PREEMPT kernel: 56 "refcount_t: addition on 0"
saturations at load, escalating to
BUG: KASAN: slab-use-after-free in rxrpc_io_thread Write of size 8
AF_RXRPC socket creation (rxrpc_create) has no capability check, so this
is reachable by an unprivileged user.
Guard rxrpc_poke_conn() with the same liveness/refcount check the sibling
rxrpc_queue_conn() uses before taking the poke reference, so a connection
past its last-active point is not poked/requeued after teardown began.
Verified before/after on KASAN+PREEMPT at equal timer volume: 56
saturations + 15 KASAN reports unpatched vs 0 and 0 patched.
Fixes: f2cce89a074e ("rxrpc: Implement a mechanism to send an event notification to a connection")
Signed-off-by: Seungwon Bae <qotmddnjs@ajou.ac.kr>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Eric Dumazet <edumazet@google.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: Simon Horman <horms@kernel.org>
cc: linux-afs@lists.infradead.org
cc: stable@vger.kernel.org
---
net/rxrpc/conn_object.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 0ece717db0f8..1be50e0c9cee 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -34,7 +34,10 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
spin_lock_irq(&local->lock);
busy = !list_empty(&conn->attend_link);
if (!busy) {
- rxrpc_get_connection(conn, why);
+ if (!rxrpc_get_connection_maybe(conn, why)) {
+ spin_unlock_irq(&local->lock);
+ return;
+ }
list_add_tail(&conn->attend_link, &local->conn_attend_q);
}
spin_unlock_irq(&local->lock);
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH net v11 17/17] rxrpc: fix use-after-free in rxrpc_poke_conn()
2026-09-23 13:37 ` [PATCH net v11 17/17] rxrpc: fix use-after-free in rxrpc_poke_conn() David Howells
@ 2026-09-27 14:59 ` netdev-bot+sashiko
0 siblings, 0 replies; 27+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27 14:59 UTC (permalink / raw)
To: dhowells
Cc: netdev, marc.dionne, kuba, davem, edumazet, pabeni, horms,
linux-afs, linux-kernel, qotmddnjs, stable
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit message misdescribes the fix.
Pre-existing issues:
- [High] Nothing drains local->conn_attend_q after the I/O thread leaves
its main loop, so a poke that lands in that window leaks the…
--
rxrpc: fix use-after-free in rxrpc_poke_conn()
This patch makes rxrpc_poke_conn() take its poke reference with
rxrpc_get_connection_maybe() in place of rxrpc_get_connection(). If the
refcount is already zero, the connection is no longer added to
local->conn_attend_q. This closes a use-after-free where the I/O thread
touched a freed connection that was still queued.
> Guard rxrpc_poke_conn() with the same liveness/refcount check the sibling
> rxrpc_queue_conn() uses before taking the poke reference, so a connection
> past its last-active point is not poked/requeued after teardown began.
[Severity: Low]
Is this paragraph accurate? rxrpc_queue_conn() checks only the active count
and takes no reference:
net/rxrpc/conn_object.c:rxrpc_queue_conn() {
if (atomic_read(&conn->active) >= 0 &&
rxrpc_queue_work(&conn->processor))
rxrpc_see_connection(conn, why);
}
The patch never reads conn->active. rxrpc_get_connection_maybe() checks
only the refcount:
if (__refcount_inc_not_zero(&conn->ref, &r))
That matches what rxrpc_poke_call() does with rxrpc_try_get_call(), not
what rxrpc_queue_conn() does.
Could the message say that the poke reference is now taken with
inc_not_zero, and drop the claim that this copies the conn->active check
in rxrpc_queue_conn()?
> diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
> index 0ece717db0f85..1be50e0c9cee5 100644
> --- a/net/rxrpc/conn_object.c
> +++ b/net/rxrpc/conn_object.c
> @@ -34,7 +34,10 @@ void rxrpc_poke_conn(struct rxrpc_connection *conn, enum rxrpc_conn_trace why)
> spin_lock_irq(&local->lock);
> busy = !list_empty(&conn->attend_link);
> if (!busy) {
> - rxrpc_get_connection(conn, why);
> + if (!rxrpc_get_connection_maybe(conn, why)) {
> + spin_unlock_irq(&local->lock);
> + return;
> + }
> list_add_tail(&conn->attend_link, &local->conn_attend_q);
[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but what
happens if a poke arrives after the I/O thread has left its main loop?
rxrpc_io_thread() breaks out of the loop once kthread_should_stop() is set
and conn_attend_q is empty. It then calls rxrpc_destroy_local():
net/rxrpc/io_thread.c:rxrpc_io_thread() {
...
if (should_stop)
break;
...
rxrpc_destroy_local(local);
...
}
rxrpc_destroy_local() sets local->dead and cleans up the client conns, the
socket and rx_queue. It never touches local->conn_attend_q.
A client connection can still have conn->timer armed at that point.
rxrpc_disconnect_client_call() arms the timer for jiffies+2, then puts the
connection on the idle list with a reference held:
net/rxrpc/conn_client.c:rxrpc_disconnect_client_call() {
...
set_bit(RXRPC_CONN_FINAL_ACK_0 + channel, &conn->flags);
rxrpc_reduce_conn_timer(conn, final_ack_at);
...
}
rxrpc_clean_up_local_conns() cancels only local->client_conn_reap_timer
synchronously. It doesn't cancel each connection's own timer before it
drops the unbundle and idle references.
If conn->timer fires in that window, the refcount is still above zero:
rxrpc_connection_timer()
rxrpc_poke_conn()
rxrpc_get_connection_maybe() <- succeeds
list_add_tail(&conn->attend_link, &local->conn_attend_q)
The only thing that consumes conn_attend_q is the loop that has already
exited. Does this leak the connection and its poke reference, along with
its references on local, peer and bundle and its key?
If so, rxnet->nr_conns never reaches zero. On netns exit,
rxrpc_destroy_all_connections() would then wait forever here:
wait_var_event(&rxnet->nr_conns, !atomic_read(&rxnet->nr_conns));
AF_RXRPC sockets can be created without privileges. An unprivileged user
could reach this window by closing the socket right after a client call
succeeds.
The old unconditional rxrpc_get_connection() had the same behaviour. The
new inc_not_zero guard doesn't help here because the refcount is nonzero.
Would rxrpc_poke_conn() need a local->dead check under local->lock to close
this? Alternatively, rxrpc_destroy_local() could drain conn_attend_q and
drop the poke references.
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923133706.1496540-1-dhowells%40redhat.com
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH net v11 10/17] rxrpc: Fix return in rxrpc_recvmsg_data() for service calls
2026-09-23 13:36 [PATCH net v11 00/17] rxrpc: Miscellaneous fixes David Howells
` (16 preceding siblings ...)
2026-09-23 13:37 ` [PATCH net v11 17/17] rxrpc: fix use-after-free in rxrpc_poke_conn() David Howells
@ 2026-09-24 8:45 ` David Howells
17 siblings, 0 replies; 27+ messages in thread
From: David Howells @ 2026-09-24 8:45 UTC (permalink / raw)
To: netdev
Cc: dhowells, Marc Dionne, Jakub Kicinski, David S. Miller,
Eric Dumazet, Paolo Abeni, Simon Horman, linux-afs, linux-kernel,
Jeffrey Altman, stable
Fixes: d001648ec7cf ("rxrpc: Don't expose skbs to in-kernel users [ver #2]")
^ permalink raw reply [flat|nested] 27+ messages in thread