Re: [PATCH net] ipvlan: unregister upper devices outside pnodes_lock

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Thu, Aug 27, 2026 at 09:27:06AM -0700, Kuniyuki Iwashima wrote:
> On Thu, Aug 27, 2026 at 6:21 AM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > syzbot reported the following circular locking dependency:
> >
> >   xs->mutex -> netdev lock -> pnodes_lock -> net->xdp.lock -> xs->mutex
> >
> > The pnodes_lock -> net->xdp.lock edge is recorded when
> > ipvlan_device_event(NETDEV_UNREGISTER) calls unregister_netdevice_many()
> > while holding pnodes_lock.  A nested NETDEV_UNREGISTER notification for
> > an IPvlan device enters xsk_notifier(), which acquires net->xdp.lock.
> >
> > Keep pnodes_lock only while marking the upper devices as dying, removing
> > them from port->ipvlans, and queueing them for unregistration.  Once the
> > devices have been detached from the protected list, release pnodes_lock
> > before unregister_netdevice_many() invokes notifier callbacks.
> >
> > The port remains alive across unregistration because
> > ipvlan_device_event() holds the reference acquired by ipvlan_port_get().
> > The dying flag prevents a concurrent ->dellink() callback from deleting a
> > queued device again.
> >
> > Fixes: 35add1093e2f ("ipvlan: Protect ipvl_port.ipvlans with mutex.")
> > Reported-by: syzbot+aa48b5fe7bfda62d1682@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=aa48b5fe7bfda62d1682
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> >  drivers/net/ipvlan/ipvlan_main.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> > index ee46a55f73d1..2730581e0a56 100644
> > --- a/drivers/net/ipvlan/ipvlan_main.c
> > +++ b/drivers/net/ipvlan/ipvlan_main.c
> > @@ -846,7 +846,6 @@ static int ipvlan_device_event(struct notifier_block *unused,
> >                                 __ipvlan_link_delete(net, ipvlan->dev, &lst_kill);
> >                 }
> >
> > -               unregister_netdevice_many(&lst_kill);
> >                 break;
> >         }
> >         case NETDEV_FEAT_CHANGE:
> > @@ -895,6 +894,10 @@ static int ipvlan_device_event(struct notifier_block *unused,
> >
> >         mutex_unlock(&port->pnodes_lock);
> >
> > +       /* Avoid invoking nested netdevice notifiers under pnodes_lock. */
> > +       if (!list_empty(&lst_kill))
> 
> nit: unregister_netdevice_many() takes care of this.

...right. I'll send v2 without it. Thanks!

> 
> 
> > +               unregister_netdevice_many(&lst_kill);
> > +
> >         ipvlan_port_put(port);
> >
> >         return ret;
> > --
> > 2.43.0
> >