Re: [PATCH net v1] net/ipv6: mcast: fix circular locking dependency in __ipv6_dev_mc_inc()

[Jiayuan Chen]

On 3/19/26 9:15 AM, Jakub Kicinski wrote:
> On Tue, 17 Mar 2026 19:12:07 +0800 Jiayuan Chen wrote:
> > syzbot reported a possible circular locking dependency:
> >
> >    fs_reclaim --> sk_lock-AF_INET6 --> &idev->mc_lock
> >
> >    CPU0                              CPU1
> >    ----                              ----
> >    lock(&idev->mc_lock)
> >                                      lock(sk_lock-AF_INET6)
> >                                      lock(&idev->mc_lock)  // blocked
> >    kzalloc(GFP_KERNEL)
> >      fs_reclaim
> >        ...nbd I/O...
> >          sk_lock-AF_INET6            // blocked -> DEADLOCK
> >
> > __ipv6_dev_mc_inc() does GFP_KERNEL allocation inside mc_lock via
> > mca_alloc(). This can enter memory reclaim, which through nbd block
> > I/O may need sk_lock-AF_INET6. But sk_lock -> mc_lock already exists
> > via setsockopt -> __ipv6_sock_mc_join, so we have a deadlock.
> >
> > Before commit 63ed8de4be81 ("mld: add mc_lock for protecting
> > per-interface mld data"), only RTNL was held during the allocation.
> > The lock ordering was always RTNL -> sk_lock (the nbd path doesn't
> > involve RTNL), so there was no circular dependency.
> >
> > Split mca_alloc() into mca_alloc() + mca_init(): mca_alloc() does the
> > GFP_KERNEL allocation before mc_lock, mca_init() initializes under
> > mc_lock. If the address already exists, the pre-allocated memory is
> > simply freed. Also move inet6_ifmcaddr_notify() outside mc_lock since
> > it also does GFP_KERNEL allocation.
> Moving the allocation seems fine, but also having to move the
> notification, potentially letting the notification go out of order
> makes me wonder if we aren't better off adding helpers for taking this
> lock which also call memalloc_noio_{save,restore} ?
Yeah, using memalloc_noio helpers is simpler. I checked and there
are about 18 places taking mc_lock, so having a common mc_lock()/mc_unlock()
wrapper that does the noio save/restore covers them all (if necessary).

The only thing that feels a bit odd is using memalloc_noio in the networking
subsystem. It makes sense in block/fs to protect itself from recursion.