[PATCH net v3 05/14] rxrpc: Fix update of call->tx_pending without holding lock
From: David Howells <dhowells@redhat.com>
Date: 2026-07-13 08:11:01
Also in:
lkml
Subsystem:
networking [general], rxrpc sockets (af_rxrpc), the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Howells, Marc Dionne, Linus Torvalds
Currently, rxrpc_send_data() updates call->tx_pending just before it
returns - but it won't be holding the call->lock 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->lock down into rxrpc_send_data() and get
rid of the indicator parameter. This makes it easier to see where the
lock is held.
(2) Make the wait_for_space path move the value in txb back into
call->tx_pending before dropping the lock prior to the wait.
(3) After waiting, if the attempt to reacquire the mutex is interrupted,
just return directly there rather than going to out_unlock
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@kernel.org
---
net/rxrpc/sendmsg.c | 52 +++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index d5060fd9631a..71343998b87d 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c@@ -320,8 +320,8 @@ 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;
@@ -334,25 +334,27 @@ 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; } 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); - return -EIO; + ret = -EIO; + 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 */
@@ -456,7 +458,7 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, if (ret == -ENOMEM) goto maybe_error; set_bit(RXRPC_CALL_TX_ERROR, &call->flags); - goto out; + goto out_txb; } if (msg_data_left(msg) == 0 && !more)
@@ -468,51 +470,58 @@ static int rxrpc_send_data(struct rxrpc_sock *rx, success: ret = copied; -out: +out_txb: 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; + call->tx_pending = NULL; + ret = call->error; + goto out_unlock; maybe_error: if (copied) { if (rxrpc_call_is_complete(call) && call->error < 0) { ret = call->error; - goto out; + goto out_unlock; } goto success; } - goto out; + goto out_txb; efault: ret = -EFAULT; - goto out; + goto out_txb; wait_for_space: ret = -EAGAIN; if (msg->msg_flags & MSG_DONTWAIT) goto maybe_error; + call->tx_pending = txb; + txb = NULL; 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; } /*
@@ -787,8 +796,8 @@ 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;
@@ -826,7 +835,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);
@@ -837,12 +845,10 @@ 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; }