[PATCH net] atm: mpoa: fix mpc->dev refcount across mpoad restart

[PATCH net] atm: mpoa: fix mpc->dev refcount across mpoad restart

[Shuvam Pandey]

atm: mpoa: fix mpc->dev refcount across mpoad restart

mpoad_close() drops the reference held in mpc->dev with dev_put(), but
the mpoa_client stays alive and keeps the same device pointer.

A later mpoad attach reuses the existing mpoa_client without
reacquiring that reference, so the next close can hit the netdevice
refcount warning. Keep the LEC device reference with the mpoa_client
until the device unregisters or the client is torn down.

Reported-by: syzbot+5ec223ccb83b24ef982f@syzkaller.appspotmail.com
Link: https://groups.google.com/g/syzkaller-bugs/c/qhZ5MJfLBOE/m/UnotmgRdAQAJ
Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
---
 net/atm/mpc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index ce8e9780373b..1e9b9c633e8b 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);
 	}
 
 	mpc->in_ops->destroy_cache(mpc);
@@ -1508,6 +1507,8 @@ static void __exit atm_mpoa_cleanup(void)
 			priv = netdev_priv(mpc->dev);
 			if (priv->lane2_ops != NULL)
 				priv->lane2_ops->associate_indicator = NULL;
+			dev_put(mpc->dev);
+			mpc->dev = NULL;
 		}
 		ddprintk("about to clear caches\n");
 		mpc->in_ops->destroy_cache(mpc);

-- 
2.50.0

Re: [PATCH net] atm: mpoa: fix mpc->dev refcount across mpoad restart

[Simon Horman]

On Tue, Apr 07, 2026 at 02:20:12PM +0545, Shuvam Pandey wrote:
> atm: mpoa: fix mpc->dev refcount across mpoad restart
> 
> mpoad_close() drops the reference held in mpc->dev with dev_put(), but
> the mpoa_client stays alive and keeps the same device pointer.
> 
> A later mpoad attach reuses the existing mpoa_client without
> reacquiring that reference, so the next close can hit the netdevice
> refcount warning. Keep the LEC device reference with the mpoa_client
> until the device unregisters or the client is torn down.

Hi Shuvam,

Including the stack trace would be useful, IMHO.

> 
> Reported-by: syzbot+5ec223ccb83b24ef982f@syzkaller.appspotmail.com
> Link: https://groups.google.com/g/syzkaller-bugs/c/qhZ5MJfLBOE/m/UnotmgRdAQAJ

A fixes tag should go here, indicating the commit which introduced
the bug - typically the commit where it first manifested.

> Signed-off-by: Shuvam Pandey <shuvampandey1@gmail.com>
> ---
>  net/atm/mpc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/atm/mpc.c b/net/atm/mpc.c
> index ce8e9780373b..1e9b9c633e8b 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);
>  	}
>  
>  	mpc->in_ops->destroy_cache(mpc);

I'm not really familiar with the object life cycle here.

But it strikes me that the purpose of dev_put() in a close callback
is to indicate the device no longer needs to be held for the connection
being closed. And, if so, I wonder if the problem here is that
there is no corresponding dev_hold() in the (unimplemented) open callback.
(I am assuming there is a 1:1 symmetry between open and close.)

> @@ -1508,6 +1507,8 @@ static void __exit atm_mpoa_cleanup(void)
>  			priv = netdev_priv(mpc->dev);
>  			if (priv->lane2_ops != NULL)
>  				priv->lane2_ops->associate_indicator = NULL;
> +			dev_put(mpc->dev);
> +			mpc->dev = NULL;
>  		}
>  		ddprintk("about to clear caches\n");
>  		mpc->in_ops->destroy_cache(mpc);

AI generated review flags that atm_mpoa_cleanup() already calls
unregister_netdevice_notifier() which will trigger the
NETDEV_UNREGISTER handler in mpoa_event_listener() which already calls
dev_put.

This seems to duplicate that. Which I expect is undesirable.