Re: [PATCH v3] nfsd: store net in svc_serv instead of deriving it from sv_permsocks

[Jeff Layton]

On Wed, 2026-10-21 at 18:12 +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 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
> 
> Rather than validating the list head before referencing it, store the
> net in the svc_serv when it is created. svc_create_pooled() is only
> used by nfsd, so add a struct net argument to it and record it in a
> new sv_net field, then use that in nfsd().
> 
> Fixes: 88c476661719 ("nfsd: pass proper net to nfsd_destroy() from NFSd kthreads")
> Reported-by: syzbot+561b859d032c8bf2fb53@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=561b859d032c8bf2fb53
> Suggested-by: NeilBrown <neil@xxxxxxxxxx>
> Tested-by: syzbot+561b859d032c8bf2fb53@xxxxxxxxxxxxxxxxxxxxxxxxx
> Reviewed-by: NeilBrown <neil@xxxxxxxxxx>
> Signed-off-by: Deepanshu Kartikey <kartikey406@xxxxxxxxx>
> ---
> v3: reword commit message, fix stray whitespace in svc_create_pooled()
>     prototype (Neil)
> v2: store the net in svc_serv via svc_create_pooled() instead of
>     walking sv_permsocks under sv_lock (Jeff, Neil)
> ---
>  fs/nfsd/nfssvc.c           | 5 ++---
>  include/linux/sunrpc/svc.h | 2 ++
>  net/sunrpc/svc.c           | 3 +++
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 2edf716ea022..06a756d5636c 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -620,7 +620,7 @@ int nfsd_create_serv(struct net *net)
>  		nfsd_max_blksize = nfsd_get_default_max_blksize();
>  	nfsd_reset_versions(nn);
>  	serv = svc_create_pooled(nfsd_programs, ARRAY_SIZE(nfsd_programs),
> -				 &nn->nfsd_svcstats,
> +				 &nn->nfsd_svcstats, net,
>  				 nfsd_max_blksize, nfsd);
>  	if (serv == NULL) {
>  		percpu_ref_exit(&nn->nfsd_net_ref);
> @@ -890,8 +890,7 @@ 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 net *net = rqstp->rq_server->sv_net;
>  	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>  	struct nfsd_thread_local_info ntli = { };
>  	bool have_mutex = false;
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 2db1b9ec5658..49a5052b2670 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -72,6 +72,7 @@ enum {
>  struct svc_serv {
>  	struct svc_program *	sv_programs;	/* RPC programs */
>  	struct svc_stat *	sv_stats;	/* RPC statistics */
> +	struct net *sv_net;		/* only set for pooled (nfsd) services */
>  	spinlock_t		sv_lock;
>  	unsigned int		sv_nprogs;	/* Number of sv_programs */
>  	unsigned int		sv_nrthreads;	/* # of running server threads */
> @@ -461,6 +462,7 @@ void		   svc_exit_thread(struct svc_rqst *);
>  struct svc_serv *  svc_create_pooled(struct svc_program *prog,
>  				     unsigned int nprog,
>  				     struct svc_stat *stats,
> +				      struct net *net,
>  				     unsigned int bufsize,
>  				     int (*threadfn)(void *data));
>  int		   svc_set_pool_threads(struct svc_serv *serv, struct svc_pool *pool,
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 8297bad2b177..810c2db27fe3 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -481,6 +481,7 @@ EXPORT_SYMBOL_GPL(svc_create);
>   * @prog:  Array of RPC programs the new service will handle
>   * @nprogs: Number of programs in the array
>   * @stats: the stats struct if desired
> + * @net: network namespace the service belongs to
>   * @bufsize: maximum message size for @prog
>   * @threadfn: a function to service RPC requests for @prog
>   *
> @@ -489,6 +490,7 @@ EXPORT_SYMBOL_GPL(svc_create);
>  struct svc_serv *svc_create_pooled(struct svc_program *prog,
>  				   unsigned int nprogs,
>  				   struct svc_stat *stats,
> +				   struct net *net,
>  				   unsigned int bufsize,
>  				   int (*threadfn)(void *data))
>  {
> @@ -502,6 +504,7 @@ struct svc_serv *svc_create_pooled(struct svc_program *prog,
>  	if (!serv)
>  		goto out_err;
>  	serv->sv_is_pooled = true;
> +	serv->sv_net = net;
>  	return serv;
>  out_err:
>  	svc_pool_map_put();

Much nicer.

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>