Re: [PATCH net v2] atm: mpoa: keep mpc->dev referenced across mpoad restart

[Paolo Abeni]

On 4/11/26 1:59 PM, Shuvam Pandey wrote:
> atm: mpoa: keep mpc->dev referenced across mpoad restart
> 
> syzbot reported a netdevice refcount warning:
> 
> refcount_t: decrement hit 0; leaking memory.
> WARNING: lib/refcount.c:31 at refcount_warn_saturate+0x70/0x110
> ...
> dev_put include/linux/netdevice.h:4466 [inline]
> mpoad_close+0x1fc/0x3e0 net/atm/mpc.c:889

The full decoded backtrace is preferred to a small excerpt

> mpoad_close() drops the reference held in mpc->dev, but the mpoa_client
> itself stays alive and keeps the same device pointer.
> 
> When mpoad is attached again, atm_mpoa_mpoad_attach() reuses the existing
> mpoa_client and its mpc->dev without reacquiring that reference, so the
> next close can hit the netdevice refcount warning.
> 
> This reference is owned by the mpoa_client/LEC association rather than a
> single mpoad open/close cycle. It is acquired when the client gets its
> LEC device and is released later from mpoa_event_listener() on
> NETDEV_UNREGISTER. Fix the imbalance by removing the dev_put() from
> mpoad_close().
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+5ec223ccb83b24ef982f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Link: https://groups.google.com/g/syzkaller-bugs/c/qhZ5MJfLBOE/m/UnotmgRdAQAJ

Preferred link is to the syzbot console:
https://syzkaller.appspot.com/bug?extid=5ec223ccb83b24ef982f

> Signed-off-by: Shuvam Pandey <shuvampandey1@xxxxxxxxx>
> ---
> Changes in v2:
> - drop the atm_mpoa_cleanup() dev_put()/NULL hunk
> - add the syzbot warning excerpt
> - add a Fixes tag
> - clarify that the final dev_put() comes from the notifier path
> 
>  net/atm/mpc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/atm/mpc.c b/net/atm/mpc.c
> index ce8e9780373b9..90ab8f2889734 100644
> --- a/net/atm/mpc.c
> +++ b/net/atm/mpc.c
> @@ -886,7 +886,6 @@ static void mpoad_close(struct atm_vcc *vcc)
>  		struct lec_priv *priv = netdev_priv(mpc->dev);
>  		priv->lane2_ops->associate_indicator = NULL;
>  		stop_mpc(mpc);
> -		dev_put(mpc->dev);

Sashiko noted a possible regression introduced by this change:

Since this patch removes the dev_put(mpc->dev) here to defer the
netdevice reference release to the NETDEV_UNREGISTER event, does this
introduce a leak of the netdevice reference on module unload?
If the atm_mpoa module is unloaded while a lec device is still active,
atm_mpoa_cleanup() unregisters the netdevice notifier and frees all
mpoa_client structures without releasing their mpc->dev references:
net/atm/mpc.c:atm_mpoa_cleanup() {
    ...
	unregister_netdevice_notifier(&mpoa_notifier);
    ...
	while (mpc != NULL) {
		tmp = mpc->next;
		if (mpc->dev != NULL) {
			stop_mpc(mpc);
            ...
		}
        ...
		kfree(mpc->mps_macs);
		kfree(mpc);
		mpc = tmp;
	}
}
Because the notifier is unregistered, NETDEV_UNREGISTER will never be
delivered to clean up the references, which would permanently leak the
netdevice reference and prevent the interface from ever being unregistered.
Should a dev_put() be added in the module exit function atm_mpoa_cleanup()?

/P