From: Simon Horman <horms@kernel.org>
To: Gui-Dong Han <hanguidong02@outlook.com>
Cc: przemyslaw.kitszel@intel.com, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, edumazet@google.com,
netdev@vger.kernel.org, anthony.l.nguyen@intel.com,
intel-wired-lan@lists.osuosl.org, baijiaju1990@gmail.com,
kuba@kernel.org, pabeni@redhat.com, davem@davemloft.net
Subject: Re: [Intel-wired-lan] [PATCH v2] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins()
Date: Fri, 6 Sep 2024 09:18:52 +0100 [thread overview]
Message-ID: <20240906081852.GB2097826@kernel.org> (raw)
In-Reply-To: <SY8P300MB0460F0F4B5D0BC6768DCA466C0932@SY8P300MB0460.AUSP300.PROD.OUTLOOK.COM>
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: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@outlook.com>
> ---
> 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 <horms@kernel.org>
WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Gui-Dong Han <hanguidong02@outlook.com>
Cc: anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
baijiaju1990@gmail.com, stable@vger.kernel.org
Subject: Re: [PATCH v2] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins()
Date: Fri, 6 Sep 2024 09:18:52 +0100 [thread overview]
Message-ID: <20240906081852.GB2097826@kernel.org> (raw)
In-Reply-To: <SY8P300MB0460F0F4B5D0BC6768DCA466C0932@SY8P300MB0460.AUSP300.PROD.OUTLOOK.COM>
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: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@outlook.com>
> ---
> 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 <horms@kernel.org>
next prev parent reply other threads:[~2024-09-06 8:19 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-03 11:48 [Intel-wired-lan] [PATCH v2] ice: Fix improper handling of refcount in ice_dpll_init_rclk_pins() Gui-Dong Han
2024-09-03 11:48 ` Gui-Dong Han
2024-09-06 8:18 ` Simon Horman [this message]
2024-09-06 8:18 ` Simon Horman
2024-09-11 16:54 ` [Intel-wired-lan] " Pucha, HimasekharX Reddy
2024-09-11 16:54 ` Pucha, HimasekharX Reddy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240906081852.GB2097826@kernel.org \
--to=horms@kernel.org \
--cc=anthony.l.nguyen@intel.com \
--cc=baijiaju1990@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hanguidong02@outlook.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.