Re: [PATCH] nfsd: prevent hung task in nfsd_nl_listener_set_doit()
Slawomir Stepien <[email protected]>
| Newsgroups | org.kernel.vger.linux-nfs,dev.linux.lists.syzbot,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <aotXzLmrd-B89YRg@nr200> |
On sie 20, 2026 07:37, Jeff Layton wrote: > On Thu, 2026-08-20 at 06:55 +0000, syzbot wrote: > > From: Slawomir Stepien <[email protected]> > > > > In nfsd_nl_listener_set_doit(), the kernel iterates over all > > NFSD_A_SERVER_SOCK_ADDR attributes provided in a netlink message to > > configure NFS server listeners. There is currently no limit on the number > > of attributes a user can send. > > > > For each attribute, svc_xprt_create_from_sa() is called, which may > > synchronously invoke request_module() to load the corresponding transport > > module. If a user provides a large number of invalid transport names, > > request_module() is called sequentially for each, taking a massive amount > > of time. Since this entire process occurs while holding the global > > nfsd_mutex, it blocks other tasks attempting to acquire the mutex and > > triggers a hung task timeout: > > > > My LLM latched onto the same explanation, but the request module > upcalls are actually quite quick and have some other gates on them that > make this problem hard to hit in practice. Oh, OK. Thanks for clearing that up! > The real problem is rpcbind registration, which can be quite slow. The > fix for that is to make the registration asynchronous, and to not > trigger errors on rpcbind registration failure. That's a bigger > project, but let us know if you're interested! I think I'm not that good to do it correctly and in some reasonable time. I might take a 2nd look at this and understand the root cause (also, based on your comments in your series) but I think in the end it might be too hard for me. > In any case, I've sent a series that includes a similar fix to this > one, along with some others: > > https://lore.kernel.org/linux-nfs/[email protected]/ > > I'll try to get the follow-on series out later today or tomorrow. Nice series, I've learnt something from it! I will be looking for v3! > > INFO: task blocked for more than 10 seconds. > > ... > > Call Trace: > > <TASK> > > __schedule+0x17e7/0x5630 kernel/sched/core.c:7234 > > schedule+0x164/0x2b0 kernel/sched/core.c:7326 > > schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7383 > > __mutex_lock_common kernel/locking/mutex.c:726 [inline] > > __mutex_lock+0x7bf/0x1550 kernel/locking/mutex.c:821 > > nfsd_nl_version_get_doit+0x17c/0xd20 fs/nfsd/nfsctl.c:1889 > > genl_family_rcv_msg_doit+0x233/0x340 net/netlink/genetlink.c:1114 > > genl_family_rcv_msg net/netlink/genetlink.c:1194 [inline] > > genl_rcv_msg+0x614/0x7a0 net/netlink/genetlink.c:1209 > > ... > > 2 locks held by task/5864: > > #0: ffffffff90114168 (cb_lock){++++}-{4:4}, at: genl_rcv+0x19/0x40 > > net/netlink/genetlink.c:1217 > > #1: ffffffff8eeb1a60 (nfsd_mutex){+.+.}-{4:4}, at: > > nfsd_nl_listener_set_doit+0x135/0x1750 fs/nfsd/nfsctl.c:1964 > > > > Furthermore, the function does not break out of the loop if > > svc_xprt_create_from_sa() fails, and it suffers from an O(N^2) complexity > > issue because svc_find_listener() iterates over the serv->sv_permsocks list > > for each attribute. > > > > Address this by introducing a hard limit of 128 NFSD_A_SERVER_SOCK_ADDR > > attributes per netlink message. The limit is enforced before acquiring the > > nfsd_mutex to prevent lock contention. Additionally, modify the loop to > > break immediately if listener creation fails, avoiding needless sequential > > request_module() calls for invalid configurations. > > > > Fixes: 16a471177496 ("NFSD: add listener-{set,get} netlink command") > > Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=41bc60511c2884783c27 > > Link: https://syzkaller.appspot.com/ai_job?id=30871f87-7a83-46b8-ab83-919894b0787b > > Signed-off-by: Slawomir Stepien <[email protected]> > > > > --- > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > > index fa92e31d1..516c04c14 100644 > > --- a/fs/nfsd/nfsctl.c > > +++ b/fs/nfsd/nfsctl.c > > @@ -1943,6 +1943,8 @@ int nfsd_nl_version_get_doit(struct sk_buff *skb, struct genl_info *info) > > return err; > > } > > > > +#define NFSD_MAX_LISTENERS 128 > > + > > /** > > * nfsd_nl_listener_set_doit - set the nfs running sockets > > * @skb: reply buffer > > @@ -1955,12 +1957,19 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info) > > struct net *net = genl_info_net(info); > > struct svc_xprt *xprt, *tmp; > > const struct nlattr *attr; > > + unsigned int count = 0; > > struct svc_serv *serv; > > LIST_HEAD(permsocks); > > struct nfsd_net *nn; > > bool delete = false; > > int err, rem; > > > > + nlmsg_for_each_attr_type(attr, NFSD_A_SERVER_SOCK_ADDR, info->nlhdr, > > + GENL_HDRLEN, rem) { > > + if (++count > NFSD_MAX_LISTENERS) > > + return -EINVAL; > > + } > > + > > mutex_lock(&nfsd_mutex); > > > > err = nfsd_create_serv(net); > > @@ -2073,8 +2082,10 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb, struct genl_info *info) > > ret = svc_xprt_create_from_sa(serv, xcl_name, net, sa, 0, > > current_cred()); > > /* always save the latest error */ > > - if (ret < 0) > > + if (ret < 0) { > > err = ret; > > + break; > > + } > > } > > > > if (!serv->sv_nrthreads && list_empty(&nn->nfsd_serv->sv_permsocks)) > > > > > > base-commit: 075b74841bd0065a3bda3440873c747938e69b68 > > I have a patch series in progress for this already, which also -- Slawomir Stepien