Subject: Re: [PATCH v2] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins()

From: Simon Horman <[email-masked]>
Date: Fri, 6 Sep 2024 09:18:52 +0100

On Tue, Sep 03, 2024 at 11:48:43AM +0000, Gui-Dong Han wrote:
> This patch addresses a reference count handling issue in the
> ice_dpll_init_rclk_pins() function. The function calls ice_dpll_get_pins(),
> which increments the reference count of the relevant resources. However,
> if the condition WARN_ON((!vsi || !vsi->netdev)) is met, the function
> currently returns an error without properly releasing the resources
> acquired by ice_dpll_get_pins(), leading to a reference count leak.
> 
> To resolve this, the check has been moved to the top of the function. This
> ensures that the function verifies the state before any resources are
> acquired, avoiding the need for additional resource management in the
> error path. 
> 
> This bug was identified by an experimental static analysis tool developed
> by our team. The tool specializes in analyzing reference count operations
> and detecting potential issues where resources are not properly managed.
> In this case, the tool flagged the missing release operation as a
> potential problem, which led to the development of this patch.
> 
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Cc: [email-masked]
> Signed-off-by: Gui-Dong Han <[email-masked]>
> ---
> v2:
> * In this patch v2, the check for vsi and vsi->netdev has been moved to
> the top of the function to simplify error handling and avoid the need for
> resource unwinding.
>   Thanks to Simon Horman for suggesting this improvement.

Thanks for the update,

I agree with your analysis and that the problem is introduced by
the cited commit.

Reviewed-by: Simon Horman <[email-masked]>