netdev - Re: [PATCH v2] drivers: ethernet: cpsw: fix panic when intrrupt coaleceing is set via ethtool

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ecdd60a-8e77-a517-aafd-6b7aacb5e0ea@ti.com>
Date:   Wed, 23 Mar 2022 16:40:30 +0530
From:   Vignesh Raghavendra <vigneshr@...com>
To:     Sondhauß, Jan <Jan.Sondhauss@...o.com>,
        "grygorii.strashko@...com" <grygorii.strashko@...com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>
Subject: Re: [PATCH v2] drivers: ethernet: cpsw: fix panic when intrrupt
 coaleceing is set via ethtool



On 23/03/22 2:17 pm, Sondhauß, Jan wrote:
> cpsw_ethtool_begin directly returns the result of pm_runtime_get_sync
> when successful.
> pm_runtime_get_sync returns -error code on failure and 0 on successful
> resume but also 1 when the device is already active. So the common case
> for cpsw_ethtool_begin is to return 1. That leads to inconsistent calls
> to pm_runtime_put in the call-chain so that pm_runtime_put is called
> one too many times and as result leaving the cpsw dev behind suspended.
> 
> The suspended cpsw dev leads to an access violation later on by
> different parts of the cpsw driver.
> 
> Fix this by calling the return-friendly pm_runtime_resume_and_get
> function.
> 
> Signed-off-by: Jan Sondhauss <jan.sondhauss@...o.com>
> ---
>  drivers/net/ethernet/ti/cpsw_ethtool.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw_ethtool.c b/drivers/net/ethernet/ti/cpsw_ethtool.c
> index 158c8d3793f4..b5bae6324970 100644
> --- a/drivers/net/ethernet/ti/cpsw_ethtool.c
> +++ b/drivers/net/ethernet/ti/cpsw_ethtool.c
> @@ -364,11 +364,9 @@ int cpsw_ethtool_op_begin(struct net_device *ndev)
>  	struct cpsw_common *cpsw = priv->cpsw;
>  	int ret;
>  
> -	ret = pm_runtime_get_sync(cpsw->dev);
> -	if (ret < 0) {
> +	ret = pm_runtime_resume_and_get(cpsw->dev);
> +	if (ret < 0)
>  		cpsw_err(priv, drv, "ethtool begin failed %d\n", ret);
> -		pm_runtime_put_noidle(cpsw->dev);
> -	}
>  
>  	return ret;
>  }

Thanks for the fix!

Reviewed-by: Vignesh Raghavendra <vigneshr@...com>

I am guessing this issue started with d43c65b05b84 (ethtool:
runtime-resume netdev parent in ethnl_ops_begin) ? If so, could you add
a Fixes tag so that patch gets backported to all affected stable kernels?

Regards
Vignesh