Re: [PATCH] ARM: at91: pm: fix imbalanced reference counter for ethernet devices

[Claudiu.Beznea]

On 18.05.2023 09:25, Claudiu Beznea wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The of_find_device_by_node() function is returning a struct platform_device
> object with the embedded struct device member's reference counter
> incremented. This needs to be dropped when done with the platform device
> returned by of_find_device_by_node().
> 
> at91_pm_eth_quirk_is_valid() calls of_find_device_by_node() on
> suspend and resume path. On suspend it calls of_find_device_by_node() and
> on resume and failure paths it drops the counter of
> struct platform_device::dev.
> 
> In case ethernet device may not wakeup there is a put_device() on
> at91_pm_eth_quirk_is_valid() which is wrong as it colides with
> put_device() on resume path leading to the reference counter of struct
> device embedded in struct platform_device to be messed, the following
> stack trace to be displayed (after 5 consecutive suspend/resume cycles)
> and the execution to hang:
> 

[ ... ]

> 
> Along with this the error path of at91_pm_config_quirks() had been also
> adapted to decrement propertly the reference counter of struct device
> embedded in struct platform_device.
> 
> Fixes: b7fc72c63399 ("ARM: at91: pm: add quirks for pm")
> Signed-off-by: Claudiu Beznea <claudiu.beznea@xxxxxxxxxxxxx>

Applied to at91-fixes (after removing useless stack traces in commit
message), thanks!

> ---
>  arch/arm/mach-at91/pm.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 60dc56d8acfb..437dd0352fd4 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -334,16 +334,14 @@ static bool at91_pm_eth_quirk_is_valid(struct at91_pm_quirk_eth *eth)
>                 pdev = of_find_device_by_node(eth->np);
>                 if (!pdev)
>                         return false;
> +               /* put_device(eth->dev) is called at the end of suspend. */
>                 eth->dev = &pdev->dev;
>         }
> 
>         /* No quirks if device isn't a wakeup source. */
> -       if (!device_may_wakeup(eth->dev)) {
> -               put_device(eth->dev);
> +       if (!device_may_wakeup(eth->dev))
>                 return false;
> -       }
> 
> -       /* put_device(eth->dev) is called at the end of suspend. */
>         return true;
>  }
> 
> @@ -439,14 +437,14 @@ static int at91_pm_config_quirks(bool suspend)
>                                 pr_err("AT91: PM: failed to enable %s clocks\n",
>                                        j == AT91_PM_G_ETH ? "geth" : "eth");
>                         }
> -               } else {
> -                       /*
> -                        * Release the reference to eth->dev taken in
> -                        * at91_pm_eth_quirk_is_valid().
> -                        */
> -                       put_device(eth->dev);
> -                       eth->dev = NULL;
>                 }
> +
> +               /*
> +                * Release the reference to eth->dev taken in
> +                * at91_pm_eth_quirk_is_valid().
> +                */
> +               put_device(eth->dev);
> +               eth->dev = NULL;
>         }
> 
>         return ret;
> --
> 2.34.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel