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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

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.