[PATCH] nfsd: fix out-of-bounds read of sv_permsocks when starting a thread

[PATCH] nfsd: fix out-of-bounds read of sv_permsocks when starting a thread

[Deepanshu Kartikey]

nfsd() derives its struct net from the first entry of
serv->sv_permsocks using list_entry() without checking that the list
is non-empty. If the list is empty, list_entry() returns a bogus
svc_xprt computed from the list head embedded in struct svc_serv, and
the following perm_sock->xpt_net load reads far past the end of the
svc_serv allocation:

  BUG: KASAN: slab-out-of-bounds in nfsd+0x118/0xbc0 fs/nfsd/nfssvc.c:894
  Read of size 8 at addr ffff88802d423f50 by task nfsd/5988

The list can be empty because nothing guarantees that a permanent
listener exists while a thread starts. With dynamic thread spawning, a
thread can begin running after the last listener has been closed and
removed.

Take sv_lock, which protects sv_permsocks, and check for an empty list
and read xpt_net in the same critical section. If there is no
permanent socket, report -ENOTCONN through svc_thread_init_status(),
which wakes the creator and exits the thread, instead of dereferencing
a bogus pointer.

Fixes: 88c476661719 ("nfsd: pass proper net to nfsd_destroy() from NFSd kthreads")
Reported-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=561b859d032c8bf2fb53
Tested-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/nfsd/nfssvc.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 2edf716ea022..da6fce79efae 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -890,17 +890,31 @@ nfsd(void *vrqstp)
 {
 	struct svc_rqst *rqstp = (struct svc_rqst *) vrqstp;
 	struct svc_pool *pool = rqstp->rq_pool;
-	struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
-	struct net *net = perm_sock->xpt_net;
-	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
+	struct svc_serv *serv = rqstp->rq_server;
+	struct svc_xprt *perm_sock;
+	struct net *net = NULL;
+	struct nfsd_net *nn = NULL;
 	struct nfsd_thread_local_info ntli = { };
 	bool have_mutex = false;
+	int err = 0;
+
+	spin_lock_bh(&serv->sv_lock);
+	if (list_empty(&serv->sv_permsocks)) {
+		err = -ENOTCONN;
+	} else {
+		perm_sock = list_first_entry(&serv->sv_permsocks,
+					     struct svc_xprt, xpt_list);
+		net = perm_sock->xpt_net;
+	}
+	spin_unlock_bh(&serv->sv_lock);
 
+	if (!err)
+		nn = net_generic(net, nfsd_net_id);
 	/* At this point, the thread shares current->fs
 	 * with the init process. We need to create files with the
 	 * umask as defined by the client instead of init's umask.
 	 */
-	svc_thread_init_status(rqstp, unshare_fs_struct());
+	svc_thread_init_status(rqstp, err ?: unshare_fs_struct());
 
 	current->fs->umask = 0;
 
-- 
2.43.0

Re: [PATCH] nfsd: fix out-of-bounds read of sv_permsocks when starting a thread

[Jeff Layton]

On Sun, 2026-10-18 at 17:03 +0530, Deepanshu Kartikey wrote:
> nfsd() derives its struct net from the first entry of
> serv->sv_permsocks using list_entry() without checking that the list
> is non-empty. If the list is empty, list_entry() returns a bogus
> svc_xprt computed from the list head embedded in struct svc_serv, and
> the following perm_sock->xpt_net load reads far past the end of the
> svc_serv allocation:
> 
>   BUG: KASAN: slab-out-of-bounds in nfsd+0x118/0xbc0 fs/nfsd/nfssvc.c:894
>   Read of size 8 at addr ffff88802d423f50 by task nfsd/5988
> 
> The list can be empty because nothing guarantees that a permanent
> listener exists while a thread starts. With dynamic thread spawning, a
> thread can begin running after the last listener has been closed and
> removed.
> 
> Take sv_lock, which protects sv_permsocks, and check for an empty list
> and read xpt_net in the same critical section. If there is no
> permanent socket, report -ENOTCONN through svc_thread_init_status(),
> which wakes the creator and exits the thread, instead of dereferencing
> a bogus pointer.
> 
> Fixes: 88c476661719 ("nfsd: pass proper net to nfsd_destroy() from NFSd kthreads")
> Reported-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=561b859d032c8bf2fb53
> Tested-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/nfsd/nfssvc.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 2edf716ea022..da6fce79efae 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -890,17 +890,31 @@ nfsd(void *vrqstp)
>  {
>  	struct svc_rqst *rqstp = (struct svc_rqst *) vrqstp;
>  	struct svc_pool *pool = rqstp->rq_pool;
> -	struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> -	struct net *net = perm_sock->xpt_net;
> -	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +	struct svc_serv *serv = rqstp->rq_server;
> +	struct svc_xprt *perm_sock;
> +	struct net *net = NULL;
> +	struct nfsd_net *nn = NULL;
>  	struct nfsd_thread_local_info ntli = { };
>  	bool have_mutex = false;
> +	int err = 0;
> +
> +	spin_lock_bh(&serv->sv_lock);
> +	if (list_empty(&serv->sv_permsocks)) {
> +		err = -ENOTCONN;
> +	} else {
> +		perm_sock = list_first_entry(&serv->sv_permsocks,
> +					     struct svc_xprt, xpt_list);
> +		net = perm_sock->xpt_net;
> +	}
> +	spin_unlock_bh(&serv->sv_lock);
> 
> 

There is another way to get to the nfsd_net here. It turns out that the
sv_stats field for nfsd always points to the one embedded in nfsd_net,
so we could avoid the socket lists altogether by just doing something
like:

  struct nfsd_net *nn = container_of(rqstp->rq_server->sv_stats,
                                   struct nfsd_net, nfsd_svcstats);

That avoids the extra locking, but it seems a little weird to depend on
the sv_stats field. Maybe we could wrap that in a little helper or
something to make it less ugly?

An alternative would be to add a serv->sv_net field, but most services
are not per-net. nfsd is the exception so it's the only one that would
use it.

>  
> +	if (!err)
> +		nn = net_generic(net, nfsd_net_id);
>  	/* At this point, the thread shares current->fs
>  	 * with the init process. We need to create files with the
>  	 * umask as defined by the client instead of init's umask.
>  	 */
> -	svc_thread_init_status(rqstp, unshare_fs_struct());
> +	svc_thread_init_status(rqstp, err ?: unshare_fs_struct());
>  
>  	current->fs->umask = 0;
>  

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH] nfsd: fix out-of-bounds read of sv_permsocks when starting a thread

[NeilBrown]

On Sun, 18 Oct 2026, Jeff Layton wrote:
> On Sun, 2026-10-18 at 17:03 +0530, Deepanshu Kartikey wrote:
> > nfsd() derives its struct net from the first entry of
> > serv->sv_permsocks using list_entry() without checking that the list
> > is non-empty. If the list is empty, list_entry() returns a bogus
> > svc_xprt computed from the list head embedded in struct svc_serv, and
> > the following perm_sock->xpt_net load reads far past the end of the
> > svc_serv allocation:
> > 
> >   BUG: KASAN: slab-out-of-bounds in nfsd+0x118/0xbc0 fs/nfsd/nfssvc.c:894
> >   Read of size 8 at addr ffff88802d423f50 by task nfsd/5988
> > 
> > The list can be empty because nothing guarantees that a permanent
> > listener exists while a thread starts. With dynamic thread spawning, a
> > thread can begin running after the last listener has been closed and
> > removed.
> > 
> > Take sv_lock, which protects sv_permsocks, and check for an empty list
> > and read xpt_net in the same critical section. If there is no
> > permanent socket, report -ENOTCONN through svc_thread_init_status(),
> > which wakes the creator and exits the thread, instead of dereferencing
> > a bogus pointer.
> > 
> > Fixes: 88c476661719 ("nfsd: pass proper net to nfsd_destroy() from NFSd kthreads")
> > Reported-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=561b859d032c8bf2fb53
> > Tested-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > ---
> >  fs/nfsd/nfssvc.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 2edf716ea022..da6fce79efae 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -890,17 +890,31 @@ nfsd(void *vrqstp)
> >  {
> >  	struct svc_rqst *rqstp = (struct svc_rqst *) vrqstp;
> >  	struct svc_pool *pool = rqstp->rq_pool;
> > -	struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> > -	struct net *net = perm_sock->xpt_net;
> > -	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > +	struct svc_serv *serv = rqstp->rq_server;
> > +	struct svc_xprt *perm_sock;
> > +	struct net *net = NULL;
> > +	struct nfsd_net *nn = NULL;
> >  	struct nfsd_thread_local_info ntli = { };
> >  	bool have_mutex = false;
> > +	int err = 0;
> > +
> > +	spin_lock_bh(&serv->sv_lock);
> > +	if (list_empty(&serv->sv_permsocks)) {
> > +		err = -ENOTCONN;
> > +	} else {
> > +		perm_sock = list_first_entry(&serv->sv_permsocks,
> > +					     struct svc_xprt, xpt_list);
> > +		net = perm_sock->xpt_net;
> > +	}
> > +	spin_unlock_bh(&serv->sv_lock);
> > 
> > 
> 
> There is another way to get to the nfsd_net here. It turns out that the
> sv_stats field for nfsd always points to the one embedded in nfsd_net,
> so we could avoid the socket lists altogether by just doing something
> like:
> 
>   struct nfsd_net *nn = container_of(rqstp->rq_server->sv_stats,
>                                    struct nfsd_net, nfsd_svcstats);
> 
> That avoids the extra locking, but it seems a little weird to depend on
> the sv_stats field. Maybe we could wrap that in a little helper or
> something to make it less ugly?
> 
> An alternative would be to add a serv->sv_net field, but most services
> are not per-net. nfsd is the exception so it's the only one that would
> use it.

Most services don't use svc_create_pooled() either - only nfsd.
I would add a 'struct net*' arg to svc_create_pooled() and
store it in serv->sv_net as you suggest.

Thanks,
NeilBrown

> 
> >  
> > +	if (!err)
> > +		nn = net_generic(net, nfsd_net_id);
> >  	/* At this point, the thread shares current->fs
> >  	 * with the init process. We need to create files with the
> >  	 * umask as defined by the client instead of init's umask.
> >  	 */
> > -	svc_thread_init_status(rqstp, unshare_fs_struct());
> > +	svc_thread_init_status(rqstp, err ?: unshare_fs_struct());
> >  
> >  	current->fs->umask = 0;
> >  
> 
> -- 
> Jeff Layton <jlayton@kernel.org>
> 
> 

Re: [PATCH] nfsd: fix out-of-bounds read of sv_permsocks when starting a thread

[Deepanshu Kartikey]

On Mon, Oct 19, 2026 at 2:15 AM NeilBrown <neilb@ownmail.net> wrote:
>
> On Sun, 18 Oct 2026, Jeff Layton wrote:
> > On Sun, 2026-10-18 at 17:03 +0530, Deepanshu Kartikey wrote:
> > > nfsd() derives its struct net from the first entry of
> > > serv->sv_permsocks using list_entry() without checking that the list
> > > is non-empty. If the list is empty, list_entry() returns a bogus
> > > svc_xprt computed from the list head embedded in struct svc_serv, and
> > > the following perm_sock->xpt_net load reads far past the end of the
> > > svc_serv allocation:
> > >
> > >   BUG: KASAN: slab-out-of-bounds in nfsd+0x118/0xbc0 fs/nfsd/nfssvc.c:894
> > >   Read of size 8 at addr ffff88802d423f50 by task nfsd/5988
> > >
> > > The list can be empty because nothing guarantees that a permanent
> > > listener exists while a thread starts. With dynamic thread spawning, a
> > > thread can begin running after the last listener has been closed and
> > > removed.
> > >
> > > Take sv_lock, which protects sv_permsocks, and check for an empty list
> > > and read xpt_net in the same critical section. If there is no
> > > permanent socket, report -ENOTCONN through svc_thread_init_status(),
> > > which wakes the creator and exits the thread, instead of dereferencing
> > > a bogus pointer.
> > >
> > > Fixes: 88c476661719 ("nfsd: pass proper net to nfsd_destroy() from NFSd kthreads")
> > > Reported-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=561b859d032c8bf2fb53
> > > Tested-by: syzbot+561b859d032c8bf2fb53@syzkaller.appspotmail.com
> > > Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> > > ---
> > >  fs/nfsd/nfssvc.c | 22 ++++++++++++++++++----
> > >  1 file changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > > index 2edf716ea022..da6fce79efae 100644
> > > --- a/fs/nfsd/nfssvc.c
> > > +++ b/fs/nfsd/nfssvc.c
> > > @@ -890,17 +890,31 @@ nfsd(void *vrqstp)
> > >  {
> > >     struct svc_rqst *rqstp = (struct svc_rqst *) vrqstp;
> > >     struct svc_pool *pool = rqstp->rq_pool;
> > > -   struct svc_xprt *perm_sock = list_entry(rqstp->rq_server->sv_permsocks.next, typeof(struct svc_xprt), xpt_list);
> > > -   struct net *net = perm_sock->xpt_net;
> > > -   struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > > +   struct svc_serv *serv = rqstp->rq_server;
> > > +   struct svc_xprt *perm_sock;
> > > +   struct net *net = NULL;
> > > +   struct nfsd_net *nn = NULL;
> > >     struct nfsd_thread_local_info ntli = { };
> > >     bool have_mutex = false;
> > > +   int err = 0;
> > > +
> > > +   spin_lock_bh(&serv->sv_lock);
> > > +   if (list_empty(&serv->sv_permsocks)) {
> > > +           err = -ENOTCONN;
> > > +   } else {
> > > +           perm_sock = list_first_entry(&serv->sv_permsocks,
> > > +                                        struct svc_xprt, xpt_list);
> > > +           net = perm_sock->xpt_net;
> > > +   }
> > > +   spin_unlock_bh(&serv->sv_lock);
> > >
> > >
> >
> > There is another way to get to the nfsd_net here. It turns out that the
> > sv_stats field for nfsd always points to the one embedded in nfsd_net,
> > so we could avoid the socket lists altogether by just doing something
> > like:
> >
> >   struct nfsd_net *nn = container_of(rqstp->rq_server->sv_stats,
> >                                    struct nfsd_net, nfsd_svcstats);
> >
> > That avoids the extra locking, but it seems a little weird to depend on
> > the sv_stats field. Maybe we could wrap that in a little helper or
> > something to make it less ugly?
> >
> > An alternative would be to add a serv->sv_net field, but most services
> > are not per-net. nfsd is the exception so it's the only one that would
> > use it.
>
> Most services don't use svc_create_pooled() either - only nfsd.
> I would add a 'struct net*' arg to svc_create_pooled() and
> store it in serv->sv_net as you suggest.
>
> Thanks,
> NeilBrown
>
> >
> > >
> > > +   if (!err)
> > > +           nn = net_generic(net, nfsd_net_id);
> > >     /* At this point, the thread shares current->fs
> > >      * with the init process. We need to create files with the
> > >      * umask as defined by the client instead of init's umask.
> > >      */
> > > -   svc_thread_init_status(rqstp, unshare_fs_struct());
> > > +   svc_thread_init_status(rqstp, err ?: unshare_fs_struct());
> > >
> > >     current->fs->umask = 0;
> > >
> >
> > --
> > Jeff Layton <jlayton@kernel.org>
> >
> >
>


Thanks Jeff and Neil, for the review. I will make the changes and
submit patch v2 shortly.

Thanks

Deepanshu