* [PATCH 1/2] SUNRPC: release lower rpc_clnt if killed waiting for XPRT_LOCKED
2026-05-04 10:28 [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Chuck Lever
@ 2026-05-04 10:28 ` Chuck Lever
2026-05-04 10:28 ` [PATCH 2/2] SUNRPC: pin upper rpc_clnt across the TLS connect_worker Chuck Lever
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2026-05-04 10:28 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: linux-nfs, netdev, Chuck Lever
From: Chuck Lever <chuck.lever@oracle.com>
xs_tcp_tls_setup_socket() creates a temporary "lower" rpc_clnt with
rpc_create() to drive the inner TLS handshake, then waits for
XPRT_LOCKED on its xprt with TASK_KILLABLE so a stuck handshake can
be aborted by signal. When the wait is interrupted, the function
jumps to out_unlock without releasing lower_clnt. The success path
and the out_close error path both call
rpc_shutdown_client(lower_clnt); only the killed-wait path skips it,
leaking the clnt and its underlying xprt.
Call rpc_shutdown_client() on this path before joining out_unlock.
xprt_release_write() is not needed here because XPRT_LOCKED was
never acquired.
Fixes: 26e8bfa30dac ("SUNRPC/TLS: Lock the lower_xprt during the tls handshake")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
net/sunrpc/xprtsock.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 2e1fe6013361..3eccd4923e6c 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2734,8 +2734,11 @@ static void xs_tcp_tls_setup_socket(struct work_struct *work)
lower_xprt = rcu_dereference(lower_clnt->cl_xprt);
rcu_read_unlock();
- if (wait_on_bit_lock(&lower_xprt->state, XPRT_LOCKED, TASK_KILLABLE))
+ if (wait_on_bit_lock(&lower_xprt->state, XPRT_LOCKED, TASK_KILLABLE)) {
+ /* XPRT_LOCKED was never acquired. */
+ rpc_shutdown_client(lower_clnt);
goto out_unlock;
+ }
status = xs_tls_handshake_sync(lower_xprt, &upper_xprt->xprtsec);
if (status) {
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] SUNRPC: pin upper rpc_clnt across the TLS connect_worker
2026-05-04 10:28 [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Chuck Lever
2026-05-04 10:28 ` [PATCH 1/2] SUNRPC: release lower rpc_clnt if killed waiting for XPRT_LOCKED Chuck Lever
@ 2026-05-04 10:28 ` Chuck Lever
2026-05-04 22:09 ` [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Michael Nemanov
2026-06-24 16:52 ` Chuck Lever
3 siblings, 0 replies; 6+ messages in thread
From: Chuck Lever @ 2026-05-04 10:28 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: linux-nfs, netdev, Chuck Lever, Michael Nemanov
From: Chuck Lever <chuck.lever@oracle.com>
The TLS connect path has a use-after-free: nothing pins the
upper rpc_clnt across the delayed connect_worker. xs_connect()
stores task->tk_client in sock_xprt::clnt as a raw pointer
and queues the worker; for TLS-secured transports that worker
is xs_tcp_tls_setup_socket(), which reads several fields out
of the saved pointer (cl_timeout, cl_program, cl_prog,
cl_vers, cl_cred, cl_stats) to construct the args for the
inner handshake rpc_clnt.
The xprt does not reference the rpc_clnt; the rpc_clnt
references the xprt. xs_destroy() does cancel the
connect_worker, but it runs only when the xprt's refcount
drops to zero, which cannot happen until the rpc_clnt
releases its cl_xprt reference in rpc_free_client_work().
When a TLS handshake fails fatally (for example, an mTLS
mount whose client cert does not match the server), the
connecting task is woken with -EACCES and exits, the mount
caller invokes rpc_shutdown_client(), and the upper rpc_clnt
is freed before the queued connect_worker fires.
xs_tcp_tls_setup_socket() then dereferences the freed clnt,
producing the refcount_t underflow Michael Nemanov reported.
Take a reference on the upper rpc_clnt in xs_connect() for
TLS transports via a new rpc_hold_client() helper, and drop
it in the connect_worker's exit path with rpc_release_client().
The xprt_lock_connect() / xprt_unlock_connect() pairing
already serialises xs_connect() with xs_tcp_tls_setup_socket(),
so the take and release are balanced one-for-one.
The non-TLS connect worker (xs_tcp_setup_socket) never reads
sock_xprt::clnt, so leave that path alone and avoid the
clnt-holds-xprt-holds-clnt cycle that would otherwise prevent
xprt destruction.
Reported-by: Michael Nemanov <michael.nemanov@vastdata.com>
Closes: https://lore.kernel.org/linux-nfs/40e3d522-dfcf-4fc1-9c55-b5e81f1536d5@vastdata.com/
Fixes: 75eb6af7acdf ("SUNRPC: Add a TCP-with-TLS RPC transport class")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
include/linux/sunrpc/clnt.h | 1 +
net/sunrpc/clnt.c | 19 +++++++++++++++++--
net/sunrpc/xprtsock.c | 11 ++++++++++-
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index f8b406b0a1af..3c2b8c355ab3 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -190,6 +190,7 @@ int rpc_switch_client_transport(struct rpc_clnt *,
const struct rpc_timeout *);
void rpc_shutdown_client(struct rpc_clnt *);
+void rpc_hold_client(struct rpc_clnt *);
void rpc_release_client(struct rpc_clnt *);
void rpc_task_release_transport(struct rpc_task *);
void rpc_task_release_client(struct rpc_task *);
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index bc8ca470718b..efa26899bc7d 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1026,8 +1026,23 @@ rpc_free_auth(struct rpc_clnt *clnt)
return NULL;
}
-/*
- * Release reference to the RPC client
+/**
+ * rpc_hold_client - acquire a reference on an rpc_clnt
+ * @clnt: rpc_clnt to pin
+ *
+ * Pairs with rpc_release_client().
+ */
+void rpc_hold_client(struct rpc_clnt *clnt)
+{
+ refcount_inc(&clnt->cl_count);
+}
+
+/**
+ * rpc_release_client - release a reference on an rpc_clnt
+ * @clnt: rpc_clnt to release
+ *
+ * Pairs with rpc_hold_client(). The rpc_clnt's resources are
+ * freed once its reference count drops to zero.
*/
void
rpc_release_client(struct rpc_clnt *clnt)
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 3eccd4923e6c..359407aae03e 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2761,6 +2761,7 @@ static void xs_tcp_tls_setup_socket(struct work_struct *work)
out_unlock:
current_restore_flags(pflags, PF_MEMALLOC);
upper_transport->clnt = NULL;
+ rpc_release_client(upper_clnt);
xprt_unlock_connect(upper_xprt, upper_transport);
return;
@@ -2808,7 +2809,15 @@ static void xs_connect(struct rpc_xprt *xprt, struct rpc_task *task)
} else
dprintk("RPC: xs_connect scheduled xprt %p\n", xprt);
- transport->clnt = task->tk_client;
+ /*
+ * Only the TLS connect_worker reads transport->clnt; pinning
+ * the upper rpc_clnt unconditionally would form a cycle with
+ * cl_xprt and prevent xprt destruction.
+ */
+ if (xprt->xprtsec.policy != RPC_XPRTSEC_NONE) {
+ rpc_hold_client(task->tk_client);
+ transport->clnt = task->tk_client;
+ }
queue_delayed_work(xprtiod_workqueue,
&transport->connect_worker,
delay);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS
2026-05-04 10:28 [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Chuck Lever
2026-05-04 10:28 ` [PATCH 1/2] SUNRPC: release lower rpc_clnt if killed waiting for XPRT_LOCKED Chuck Lever
2026-05-04 10:28 ` [PATCH 2/2] SUNRPC: pin upper rpc_clnt across the TLS connect_worker Chuck Lever
@ 2026-05-04 22:09 ` Michael Nemanov
2026-06-24 16:52 ` Chuck Lever
3 siblings, 0 replies; 6+ messages in thread
From: Michael Nemanov @ 2026-05-04 22:09 UTC (permalink / raw)
To: Chuck Lever, Trond Myklebust, Anna Schumaker, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: linux-nfs, netdev, Chuck Lever
On 04/05/2026 13:28, Chuck Lever wrote:
> Patch 2 fixes a use-after-free Michael Nemanov hit on an mTLS mount
> whose client certificate the server rejected.
Reviewed and tested both patches.
Confirmed 3-sec delayed work was happening multiple times without UAF.
Thank you.
Tested-by: Michael Nemanov <michael.nemanov@vastdata.com>
Reviewed-by: Michael Nemanov <michael.nemanov@vastdata.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS
2026-05-04 10:28 [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Chuck Lever
` (2 preceding siblings ...)
2026-05-04 22:09 ` [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS Michael Nemanov
@ 2026-06-24 16:52 ` Chuck Lever
2026-06-24 17:41 ` Anna Schumaker
3 siblings, 1 reply; 6+ messages in thread
From: Chuck Lever @ 2026-06-24 16:52 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: linux-nfs, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Nemanov, Chuck Lever
Gentle ping on this series, posted seven weeks ago. Michael
Nemanov reviewed and tested both patches the following day; he is
the reporter of the use-after-free that patch 2 addresses on an
mTLS mount whose client certificate the server rejected.
Could one of you queue these for an upcoming release? I am glad
to repost against a current base if that is easier to apply.
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] Fix a few memory bugs in RPC-with-TLS
2026-06-24 16:52 ` Chuck Lever
@ 2026-06-24 17:41 ` Anna Schumaker
0 siblings, 0 replies; 6+ messages in thread
From: Anna Schumaker @ 2026-06-24 17:41 UTC (permalink / raw)
To: Chuck Lever, Trond Myklebust
Cc: linux-nfs, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Nemanov, Chuck Lever
Hi Chuck,
On Wed, Jun 24, 2026, at 12:52 PM, Chuck Lever wrote:
> Gentle ping on this series, posted seven weeks ago. Michael
> Nemanov reviewed and tested both patches the following day; he is
> the reporter of the use-after-free that patch 2 addresses on an
> mTLS mount whose client certificate the server rejected.
>
> Could one of you queue these for an upcoming release? I am glad
> to repost against a current base if that is easier to apply.
I don't remember seeing these patches when they came in initially. Sorry
about that! I'll take a look soon, and try to include them in a bugfixes
pull request.
Anna
>
> --
> Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread