netdev - Re: [PATCH] net: stmmac:fix system hang when setting up tag

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: <de8e3387-6224-d367-d897-a4f61450cd7e@intel.com>
Date:   Thu, 13 Apr 2023 10:00:41 -0700
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Yan Wang <rk.code@...look.com>, <davem@...emloft.net>,
        <edumazet@...gle.com>, <pabeni@...hat.com>, <kuba@...nel.org>,
        <mcoquelin.stm32@...il.com>
CC:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        "open list:STMMAC ETHERNET DRIVER" <netdev@...r.kernel.org>,
        "moderated list:ARM/STM32 ARCHITECTURE" 
        <linux-stm32@...md-mailman.stormreply.com>,
        "moderated list:ARM/STM32 ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: stmmac:fix system hang when setting up tag_8021q
 VLAN for DSA ports



On 4/13/2023 6:38 AM, Yan Wang wrote:
> The system hang because of dsa_tag_8021q_port_setup()->
> 				stmmac_vlan_rx_add_vid().
> 
> I found in stmmac_drv_probe() that cailing pm_runtime_put()
> disabled the clock.
> 
> First, when the kernel is compiled with CONFIG_PM=y,The stmmac's
> resume/suspend is active.
> 
> Secondly,stmmac as DSA master,the dsa_tag_8021q_port_setup() function
> will callback stmmac_vlan_rx_add_vid when DSA dirver starts. However,
> The system is hanged for the stmmac_vlan_rx_add_vid() accesses its
> registers after stmmac's clock is closed.
> 

And access to the registers basically just waits because its clock is
stopped?

> I would suggest adding the pm_runtime_resume_and_get() to the
> stmmac_vlan_rx_add_vid().This guarantees that resuming clock output
> while in use.
> 

It feels a bit odd to need this within the VLAN function, but these can
be entered at an odd time where its possible that the clock has been
closed... Ok.

> Signed-off-by: Yan Wang <rk.code@...look.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d7fcab057032..f9cd063f1fe3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6350,6 +6350,10 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
>  	bool is_double = false;
>  	int ret;
>  
> +	ret = pm_runtime_resume_and_get(priv->device);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (be16_to_cpu(proto) == ETH_P_8021AD)
>  		is_double = true;
>  
> @@ -6357,16 +6361,18 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
>  	ret = stmmac_vlan_update(priv, is_double);
>  	if (ret) {
>  		clear_bit(vid, priv->active_vlans);
> -		return ret;
> +		goto err_pm_put;
>  	}
>  
>  	if (priv->hw->num_vlan) {
>  		ret = stmmac_add_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  	}
> +err_pm_put:
> +	pm_runtime_put(priv->device);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vid)