[PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Chuck Lever]

svc_age_temp_xprts_now() moves each matching temporary transport
onto a local list under sv_lock, then drops the lock and walks the
local list without holding a reference on any of them. In the
meantime svc_delete_xprt() can take sv_lock, unlink the same
xpt_list node, and drop the transport's last reference. An idle
temporary transport holds only that one reference, so a peer FIN
that arrives between the two loops frees the svc_sock out from under
the second loop. The notifier then sets XPT_CLOSE in freed memory,
and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
object and queues that object on a pool for an nfsd thread. KASAN
reports a slab-use-after-free in svc_xprt_enqueue() under
nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
an nfsd thread.

The two-phase walk dates from when the second loop called into the
socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
call sleeping functions from the notifier block callbacks") moved
that work to the nfsd threads, so nothing left in the second loop
needs sv_lock dropped. Mark and enqueue each matching transport
under sv_lock in a single pass, as svc_age_temp_xprts() already
does, so that svc_delete_xprt() cannot unlink and free a transport
the notifier is still touching.

Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 net/sunrpc/svc_xprt.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Mohammed, thanks for the report. Please test and report your results.

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 5db42e53be62..076c624fe675 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
 {
 	struct svc_xprt *xprt;
 	struct list_head *le, *next;
-	LIST_HEAD(to_be_closed);
 
 	spin_lock_bh(&serv->sv_lock);
 	list_for_each_safe(le, next, &serv->sv_tempsocks) {
 		xprt = list_entry(le, struct svc_xprt, xpt_list);
-		if (rpc_cmp_addr(server_addr, (struct sockaddr *)
-				&xprt->xpt_local)) {
-			dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
-			list_move(le, &to_be_closed);
-		}
-	}
-	spin_unlock_bh(&serv->sv_lock);
-
-	while (!list_empty(&to_be_closed)) {
-		le = to_be_closed.next;
+		if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
+				&xprt->xpt_local))
+			continue;
 		list_del_init(le);
-		xprt = list_entry(le, struct svc_xprt, xpt_list);
 		set_bit(XPT_CLOSE, &xprt->xpt_flags);
 		set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
 		dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
 				xprt);
 		svc_xprt_enqueue(xprt);
 	}
+	spin_unlock_bh(&serv->sv_lock);
 }
 EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
 
-- 
2.54.0

Re: [PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[NeilBrown]

On Fri, 23 Oct 2026, Chuck Lever wrote:
> svc_age_temp_xprts_now() moves each matching temporary transport
> onto a local list under sv_lock, then drops the lock and walks the
> local list without holding a reference on any of them. In the
> meantime svc_delete_xprt() can take sv_lock, unlink the same
> xpt_list node, and drop the transport's last reference. An idle
> temporary transport holds only that one reference, so a peer FIN
> that arrives between the two loops frees the svc_sock out from under
> the second loop. The notifier then sets XPT_CLOSE in freed memory,
> and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
> object and queues that object on a pool for an nfsd thread. KASAN
> reports a slab-use-after-free in svc_xprt_enqueue() under
> nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
> an nfsd thread.
> 
> The two-phase walk dates from when the second loop called into the
> socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
> call sleeping functions from the notifier block callbacks") moved
> that work to the nfsd threads, so nothing left in the second loop
> needs sv_lock dropped. Mark and enqueue each matching transport
> under sv_lock in a single pass, as svc_age_temp_xprts() already
> does, so that svc_delete_xprt() cannot unlink and free a transport
> the notifier is still touching.
> 
> Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
> Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
> Signed-off-by: Chuck Lever <cel@kernel.org>

Reviewed-by: NeilBrown <neil@brown.name>

Everything being moved into the locked region is safe in that context,
so this is a nice simplification as well as being a bug-fix.

Thanks,
NeilBrown


> ---
>  net/sunrpc/svc_xprt.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> Mohammed, thanks for the report. Please test and report your results.
> 
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 5db42e53be62..076c624fe675 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
>  {
>  	struct svc_xprt *xprt;
>  	struct list_head *le, *next;
> -	LIST_HEAD(to_be_closed);
>  
>  	spin_lock_bh(&serv->sv_lock);
>  	list_for_each_safe(le, next, &serv->sv_tempsocks) {
>  		xprt = list_entry(le, struct svc_xprt, xpt_list);
> -		if (rpc_cmp_addr(server_addr, (struct sockaddr *)
> -				&xprt->xpt_local)) {
> -			dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
> -			list_move(le, &to_be_closed);
> -		}
> -	}
> -	spin_unlock_bh(&serv->sv_lock);
> -
> -	while (!list_empty(&to_be_closed)) {
> -		le = to_be_closed.next;
> +		if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
> +				&xprt->xpt_local))
> +			continue;
>  		list_del_init(le);
> -		xprt = list_entry(le, struct svc_xprt, xpt_list);
>  		set_bit(XPT_CLOSE, &xprt->xpt_flags);
>  		set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
>  		dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
>  				xprt);
>  		svc_xprt_enqueue(xprt);
>  	}
> +	spin_unlock_bh(&serv->sv_lock);
>  }
>  EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
>  
> -- 
> 2.54.0
> 
> 

Re: [PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Mohammed]

Hi Chuck,

This patch fixes the issue.

Thanks for the quick response and fix!

Best,
Mohammed

On Thu, Oct 22, 2026 at 11:54 PM Chuck Lever <cel@kernel.org> wrote:
>
> svc_age_temp_xprts_now() moves each matching temporary transport
> onto a local list under sv_lock, then drops the lock and walks the
> local list without holding a reference on any of them. In the
> meantime svc_delete_xprt() can take sv_lock, unlink the same
> xpt_list node, and drop the transport's last reference. An idle
> temporary transport holds only that one reference, so a peer FIN
> that arrives between the two loops frees the svc_sock out from under
> the second loop. The notifier then sets XPT_CLOSE in freed memory,
> and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
> object and queues that object on a pool for an nfsd thread. KASAN
> reports a slab-use-after-free in svc_xprt_enqueue() under
> nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
> an nfsd thread.
>
> The two-phase walk dates from when the second loop called into the
> socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
> call sleeping functions from the notifier block callbacks") moved
> that work to the nfsd threads, so nothing left in the second loop
> needs sv_lock dropped. Mark and enqueue each matching transport
> under sv_lock in a single pass, as svc_age_temp_xprts() already
> does, so that svc_delete_xprt() cannot unlink and free a transport
> the notifier is still touching.
>
> Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
> Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  net/sunrpc/svc_xprt.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> Mohammed, thanks for the report. Please test and report your results.
>
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 5db42e53be62..076c624fe675 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
>  {
>         struct svc_xprt *xprt;
>         struct list_head *le, *next;
> -       LIST_HEAD(to_be_closed);
>
>         spin_lock_bh(&serv->sv_lock);
>         list_for_each_safe(le, next, &serv->sv_tempsocks) {
>                 xprt = list_entry(le, struct svc_xprt, xpt_list);
> -               if (rpc_cmp_addr(server_addr, (struct sockaddr *)
> -                               &xprt->xpt_local)) {
> -                       dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
> -                       list_move(le, &to_be_closed);
> -               }
> -       }
> -       spin_unlock_bh(&serv->sv_lock);
> -
> -       while (!list_empty(&to_be_closed)) {
> -               le = to_be_closed.next;
> +               if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
> +                               &xprt->xpt_local))
> +                       continue;
>                 list_del_init(le);
> -               xprt = list_entry(le, struct svc_xprt, xpt_list);
>                 set_bit(XPT_CLOSE, &xprt->xpt_flags);
>                 set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
>                 dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
>                                 xprt);
>                 svc_xprt_enqueue(xprt);
>         }
> +       spin_unlock_bh(&serv->sv_lock);
>  }
>  EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
>
> --
> 2.54.0
>

Re: [PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Chuck Lever]

On Mon, Oct 26, 2026, at 6:30 AM, Mohammed wrote:
> Hi Chuck,
>
> This patch fixes the issue.
>
> Thanks for the quick response and fix!

May I add:

Tested-by: Mohammed <mohammed.saad.alquraini@gmail.com>

?

> Best,
> Mohammed
>
> On Thu, Oct 22, 2026 at 11:54 PM Chuck Lever <cel@kernel.org> wrote:
>>
>> svc_age_temp_xprts_now() moves each matching temporary transport
>> onto a local list under sv_lock, then drops the lock and walks the
>> local list without holding a reference on any of them. In the
>> meantime svc_delete_xprt() can take sv_lock, unlink the same
>> xpt_list node, and drop the transport's last reference. An idle
>> temporary transport holds only that one reference, so a peer FIN
>> that arrives between the two loops frees the svc_sock out from under
>> the second loop. The notifier then sets XPT_CLOSE in freed memory,
>> and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
>> object and queues that object on a pool for an nfsd thread. KASAN
>> reports a slab-use-after-free in svc_xprt_enqueue() under
>> nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
>> an nfsd thread.
>>
>> The two-phase walk dates from when the second loop called into the
>> socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
>> call sleeping functions from the notifier block callbacks") moved
>> that work to the nfsd threads, so nothing left in the second loop
>> needs sv_lock dropped. Mark and enqueue each matching transport
>> under sv_lock in a single pass, as svc_age_temp_xprts() already
>> does, so that svc_delete_xprt() cannot unlink and free a transport
>> the notifier is still touching.
>>
>> Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
>> Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
>> Signed-off-by: Chuck Lever <cel@kernel.org>
>> ---
>>  net/sunrpc/svc_xprt.c | 16 ++++------------
>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> Mohammed, thanks for the report. Please test and report your results.
>>
>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
>> index 5db42e53be62..076c624fe675 100644
>> --- a/net/sunrpc/svc_xprt.c
>> +++ b/net/sunrpc/svc_xprt.c
>> @@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
>>  {
>>         struct svc_xprt *xprt;
>>         struct list_head *le, *next;
>> -       LIST_HEAD(to_be_closed);
>>
>>         spin_lock_bh(&serv->sv_lock);
>>         list_for_each_safe(le, next, &serv->sv_tempsocks) {
>>                 xprt = list_entry(le, struct svc_xprt, xpt_list);
>> -               if (rpc_cmp_addr(server_addr, (struct sockaddr *)
>> -                               &xprt->xpt_local)) {
>> -                       dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
>> -                       list_move(le, &to_be_closed);
>> -               }
>> -       }
>> -       spin_unlock_bh(&serv->sv_lock);
>> -
>> -       while (!list_empty(&to_be_closed)) {
>> -               le = to_be_closed.next;
>> +               if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
>> +                               &xprt->xpt_local))
>> +                       continue;
>>                 list_del_init(le);
>> -               xprt = list_entry(le, struct svc_xprt, xpt_list);
>>                 set_bit(XPT_CLOSE, &xprt->xpt_flags);
>>                 set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
>>                 dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
>>                                 xprt);
>>                 svc_xprt_enqueue(xprt);
>>         }
>> +       spin_unlock_bh(&serv->sv_lock);
>>  }
>>  EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
>>
>> --
>> 2.54.0
>>

-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Mohammed]

Sure thing.

On Mon, Oct 26, 2026 at 4:05 PM Chuck Lever <cel@kernel.org> wrote:
>
>
>
> On Mon, Oct 26, 2026, at 6:30 AM, Mohammed wrote:
> > Hi Chuck,
> >
> > This patch fixes the issue.
> >
> > Thanks for the quick response and fix!
>
> May I add:
>
> Tested-by: Mohammed <mohammed.saad.alquraini@gmail.com>
>
> ?
>
> > Best,
> > Mohammed
> >
> > On Thu, Oct 22, 2026 at 11:54 PM Chuck Lever <cel@kernel.org> wrote:
> >>
> >> svc_age_temp_xprts_now() moves each matching temporary transport
> >> onto a local list under sv_lock, then drops the lock and walks the
> >> local list without holding a reference on any of them. In the
> >> meantime svc_delete_xprt() can take sv_lock, unlink the same
> >> xpt_list node, and drop the transport's last reference. An idle
> >> temporary transport holds only that one reference, so a peer FIN
> >> that arrives between the two loops frees the svc_sock out from under
> >> the second loop. The notifier then sets XPT_CLOSE in freed memory,
> >> and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
> >> object and queues that object on a pool for an nfsd thread. KASAN
> >> reports a slab-use-after-free in svc_xprt_enqueue() under
> >> nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
> >> an nfsd thread.
> >>
> >> The two-phase walk dates from when the second loop called into the
> >> socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
> >> call sleeping functions from the notifier block callbacks") moved
> >> that work to the nfsd threads, so nothing left in the second loop
> >> needs sv_lock dropped. Mark and enqueue each matching transport
> >> under sv_lock in a single pass, as svc_age_temp_xprts() already
> >> does, so that svc_delete_xprt() cannot unlink and free a transport
> >> the notifier is still touching.
> >>
> >> Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
> >> Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
> >> Signed-off-by: Chuck Lever <cel@kernel.org>
> >> ---
> >>  net/sunrpc/svc_xprt.c | 16 ++++------------
> >>  1 file changed, 4 insertions(+), 12 deletions(-)
> >>
> >> Mohammed, thanks for the report. Please test and report your results.
> >>
> >> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> >> index 5db42e53be62..076c624fe675 100644
> >> --- a/net/sunrpc/svc_xprt.c
> >> +++ b/net/sunrpc/svc_xprt.c
> >> @@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
> >>  {
> >>         struct svc_xprt *xprt;
> >>         struct list_head *le, *next;
> >> -       LIST_HEAD(to_be_closed);
> >>
> >>         spin_lock_bh(&serv->sv_lock);
> >>         list_for_each_safe(le, next, &serv->sv_tempsocks) {
> >>                 xprt = list_entry(le, struct svc_xprt, xpt_list);
> >> -               if (rpc_cmp_addr(server_addr, (struct sockaddr *)
> >> -                               &xprt->xpt_local)) {
> >> -                       dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
> >> -                       list_move(le, &to_be_closed);
> >> -               }
> >> -       }
> >> -       spin_unlock_bh(&serv->sv_lock);
> >> -
> >> -       while (!list_empty(&to_be_closed)) {
> >> -               le = to_be_closed.next;
> >> +               if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
> >> +                               &xprt->xpt_local))
> >> +                       continue;
> >>                 list_del_init(le);
> >> -               xprt = list_entry(le, struct svc_xprt, xpt_list);
> >>                 set_bit(XPT_CLOSE, &xprt->xpt_flags);
> >>                 set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
> >>                 dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
> >>                                 xprt);
> >>                 svc_xprt_enqueue(xprt);
> >>         }
> >> +       spin_unlock_bh(&serv->sv_lock);
> >>  }
> >>  EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
> >>
> >> --
> >> 2.54.0
> >>
>
> --
> Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH] SUNRPC: Fix use-after-free in svc_age_temp_xprts_now()

[Jeff Layton]

On Thu, 2026-10-22 at 16:53 -0400, Chuck Lever wrote:
> svc_age_temp_xprts_now() moves each matching temporary transport
> onto a local list under sv_lock, then drops the lock and walks the
> local list without holding a reference on any of them. In the
> meantime svc_delete_xprt() can take sv_lock, unlink the same
> xpt_list node, and drop the transport's last reference. An idle
> temporary transport holds only that one reference, so a peer FIN
> that arrives between the two loops frees the svc_sock out from under
> the second loop. The notifier then sets XPT_CLOSE in freed memory,
> and svc_xprt_enqueue() reads xpt_flags and xpt_server from the freed
> object and queues that object on a pool for an nfsd thread. KASAN
> reports a slab-use-after-free in svc_xprt_enqueue() under
> nfsd_inetaddr_event(), with the object freed by svc_sock_free() on
> an nfsd thread.
> 
> The two-phase walk dates from when the second loop called into the
> socket layer and could sleep. Commit 546125d16142 ("sunrpc: don't
> call sleeping functions from the notifier block callbacks") moved
> that work to the nfsd threads, so nothing left in the second loop
> needs sv_lock dropped. Mark and enqueue each matching transport
> under sv_lock in a single pass, as svc_age_temp_xprts() already
> does, so that svc_delete_xprt() cannot unlink and free a transport
> the notifier is still touching.
> 
> Fixes: c3d4879e01be ("sunrpc: Add a function to close temporary transports immediately")
> Reported-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
>  net/sunrpc/svc_xprt.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> Mohammed, thanks for the report. Please test and report your results.
> 
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 5db42e53be62..076c624fe675 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -1075,29 +1075,21 @@ void svc_age_temp_xprts_now(struct svc_serv *serv, struct sockaddr *server_addr)
>  {
>  	struct svc_xprt *xprt;
>  	struct list_head *le, *next;
> -	LIST_HEAD(to_be_closed);
>  
>  	spin_lock_bh(&serv->sv_lock);
>  	list_for_each_safe(le, next, &serv->sv_tempsocks) {
>  		xprt = list_entry(le, struct svc_xprt, xpt_list);
> -		if (rpc_cmp_addr(server_addr, (struct sockaddr *)
> -				&xprt->xpt_local)) {
> -			dprintk("svc_age_temp_xprts_now: found %p\n", xprt);
> -			list_move(le, &to_be_closed);
> -		}
> -	}
> -	spin_unlock_bh(&serv->sv_lock);
> -
> -	while (!list_empty(&to_be_closed)) {
> -		le = to_be_closed.next;
> +		if (!rpc_cmp_addr(server_addr, (struct sockaddr *)
> +				&xprt->xpt_local))
> +			continue;
>  		list_del_init(le);
> -		xprt = list_entry(le, struct svc_xprt, xpt_list);
>  		set_bit(XPT_CLOSE, &xprt->xpt_flags);
>  		set_bit(XPT_KILL_TEMP, &xprt->xpt_flags);
>  		dprintk("svc_age_temp_xprts_now: queuing xprt %p for closing\n",
>  				xprt);
>  		svc_xprt_enqueue(xprt);
>  	}
> +	spin_unlock_bh(&serv->sv_lock);
>  }
>  EXPORT_SYMBOL_GPL(svc_age_temp_xprts_now);
>  

Reviewed-by: Jeff Layton <jlayton@kernel.org>