From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 13474C3DA7F for ; Sat, 10 Aug 2024 09:10:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 8AB0D41481; Sat, 10 Aug 2024 09:10:26 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id WwHp8_N68ApO; Sat, 10 Aug 2024 09:10:25 +0000 (UTC) X-Comment: SPF check N/A for local connections - client-ip=140.211.166.34; helo=ash.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver= DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8E87941466 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org; s=default; t=1723281025; bh=Ol5ANs+U/a0i2lbGU0JF63v7sVxvV2UVjcfqmERwff8=; h=Date:From:To:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: Cc:From; b=VokMly2eMo6zI50/Pf3z6MHPUnSa/07+6dUhniX699Nlad/q4EORsGwZ8rpROaZJG FbY+cKtyXNg4AcQxNcESudHv0WIEs5eRxk6GpgCR7iAvCVUYhYxU+b8/gr2if26vyZ p+COU+tyTzuifrw7EQyTYtXN8K+JMtiD1l79xFSlfSGTW5sklEt9a73gfNuNHomsyP wqg5fFDolmvHu9a4WAw9x72faz+kowiiQejYRB5N+XHLpueZj87Fkdo6Tj0l1GnETw S67kbNBJEWw31/NN34ryjpblZz7kFmJaCm5d9+KHsV4a98zgh8ukogHfzmENpY3LLm /3pLQeW/cPtVw== Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp4.osuosl.org (Postfix) with ESMTP id 8E87941466; Sat, 10 Aug 2024 09:10:25 +0000 (UTC) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by ash.osuosl.org (Postfix) with ESMTP id DB8AA1BF85D for ; Sat, 10 Aug 2024 09:10:24 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id D322580DB4 for ; Sat, 10 Aug 2024 09:10:24 +0000 (UTC) X-Virus-Scanned: amavis at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP id qbA8L27B4SPv for ; Sat, 10 Aug 2024 09:10:24 +0000 (UTC) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=145.40.73.55; helo=sin.source.kernel.org; envelope-from=horms@kernel.org; receiver= DMARC-Filter: OpenDMARC Filter v1.4.2 smtp1.osuosl.org F27E380DA9 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org F27E380DA9 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by smtp1.osuosl.org (Postfix) with ESMTPS id F27E380DA9 for ; Sat, 10 Aug 2024 09:10:23 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 4BFF8CE1736; Sat, 10 Aug 2024 09:10:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 04231C32781; Sat, 10 Aug 2024 09:10:17 +0000 (UTC) Date: Sat, 10 Aug 2024 10:10:15 +0100 From: Simon Horman To: Gui-Dong Han Message-ID: <20240810091015.GF1951@kernel.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723281020; bh=WXVJCXwzW1I64R/W1MgTk/3ACJlurBYFgx0Z/peXnpg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vG6OR5PyBUb0T2jL8vh9GqW3U7h4zQlB2AlJP6piU89z4nevU4SxV+ZrzvpXJV8c7 0DdJYWt+8M0cEeCDsKaOrN5l5/hf9Zf8R+rmlly+rgAp7Ybvrp1zhwM81SIe7cJhTF XmvIyJtSWAxHfCIOweNktdNQQKW2CI0s/D8ZZAez6IU+q7xfDjFp+LHG50EkxF0B/+ AiD39UOxZPHTxCXPOulIUPQatB8QE0usuAe5dZ3gx0NuCjrUASvmYj2ZtxKv/XdrHs R9eFKfjGgFfW8WN1WKOTyNs0dd8Tsx51yEPvS3ovPpCDFNvnLO1kfVx06+j6dgSEpN GsDyyXcTDNRog== X-Mailman-Original-Authentication-Results: smtp1.osuosl.org; dmarc=pass (p=none dis=none) header.from=kernel.org X-Mailman-Original-Authentication-Results: smtp1.osuosl.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=vG6OR5Py Subject: Re: [Intel-wired-lan] [PATCH] ice: Fix improper handling of refcount in ice_sriov_set_msix_vec_count() X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" On Fri, Aug 09, 2024 at 12:59:11PM +0800, Gui-Dong Han wrote: > This patch addresses an issue with improper reference count handling in the > ice_sriov_set_msix_vec_count() function. Specifically, the function calls > ice_get_vf_by_id(), which increments the reference count of the vf pointer. > If the subsequent call to ice_get_vf_vsi() fails, the function currently > returns an error without decrementing the reference count of the vf > pointer, leading to a reference count leak. > > The correct behavior, as implemented in this patch, is to decrement the > reference count using ice_put_vf(vf) before returning an error when vsi > is NULL. > > This bug was identified by an experimental static analysis tool developed > by our team. The tool specializes in analyzing reference count operations > and identifying potential mismanagement of reference counts. In this case, > the tool flagged the missing decrement operation as a potential issue, > leading to this patch. > > Fixes: 4035c72dc1ba ("ice: reconfig host after changing MSI-X on VF") > Cc: stable@vger.kernel.org > Signed-off-by: Gui-Dong Han Thanks Gui-Dong Han, I agree with your analysis. However, I wonder if the same resource leak can occur in the unroll label, if the if clause results in a return. It is around line 1444 without your patch applied. vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix); if (vf->first_vector_idx < 0) return -EINVAL; > --- > drivers/net/ethernet/intel/ice/ice_sriov.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c > index 55ef33208456..eb5030aba9a5 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c > @@ -1096,8 +1096,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count) > return -ENOENT; > > vsi = ice_get_vf_vsi(vf); > - if (!vsi) > + if (!vsi) { > + ice_put_vf(vf); > return -ENOENT; > + } > > prev_msix = vf->num_msix; > prev_queues = vf->num_vf_qs; > -- > 2.25.1 > > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CA615155345; Sat, 10 Aug 2024 09:10:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723281020; cv=none; b=N9lm2tn93E4+f4kc4Vpu8R/B93t4IiguzOhjeOQF229LgjDCZyyJNpl3zw07h4jCbG8GAgS3kz5vun49qdga4cLCVdevIYvuUn6EprlCMlIu48cKmzVWrKTR+57dmOnryRrSUtCprvWIMuy7vgRCb3ws2fWLF7ZOnDSyi4kqLac= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1723281020; c=relaxed/simple; bh=WXVJCXwzW1I64R/W1MgTk/3ACJlurBYFgx0Z/peXnpg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TK9Yhern3MKAlMP0BhZKQ2jU/iA+9IMea+q2atd0oiJIBRMx5clT/cFHFe6/UArg+2pk51golAJvvHI22yJVNM/t9ba5JFMqx1K62hAcfKem+oaKGE83edDuzp1LfNRvJlepUSfO/G17/JIyacLkOeRd76OTP2/r9wlGu0kWjSw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=vG6OR5Py; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="vG6OR5Py" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 04231C32781; Sat, 10 Aug 2024 09:10:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723281020; bh=WXVJCXwzW1I64R/W1MgTk/3ACJlurBYFgx0Z/peXnpg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vG6OR5PyBUb0T2jL8vh9GqW3U7h4zQlB2AlJP6piU89z4nevU4SxV+ZrzvpXJV8c7 0DdJYWt+8M0cEeCDsKaOrN5l5/hf9Zf8R+rmlly+rgAp7Ybvrp1zhwM81SIe7cJhTF XmvIyJtSWAxHfCIOweNktdNQQKW2CI0s/D8ZZAez6IU+q7xfDjFp+LHG50EkxF0B/+ AiD39UOxZPHTxCXPOulIUPQatB8QE0usuAe5dZ3gx0NuCjrUASvmYj2ZtxKv/XdrHs R9eFKfjGgFfW8WN1WKOTyNs0dd8Tsx51yEPvS3ovPpCDFNvnLO1kfVx06+j6dgSEpN GsDyyXcTDNRog== Date: Sat, 10 Aug 2024 10:10:15 +0100 From: Simon Horman To: Gui-Dong Han 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] ice: Fix improper handling of refcount in ice_sriov_set_msix_vec_count() Message-ID: <20240810091015.GF1951@kernel.org> References: Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Fri, Aug 09, 2024 at 12:59:11PM +0800, Gui-Dong Han wrote: > This patch addresses an issue with improper reference count handling in the > ice_sriov_set_msix_vec_count() function. Specifically, the function calls > ice_get_vf_by_id(), which increments the reference count of the vf pointer. > If the subsequent call to ice_get_vf_vsi() fails, the function currently > returns an error without decrementing the reference count of the vf > pointer, leading to a reference count leak. > > The correct behavior, as implemented in this patch, is to decrement the > reference count using ice_put_vf(vf) before returning an error when vsi > is NULL. > > This bug was identified by an experimental static analysis tool developed > by our team. The tool specializes in analyzing reference count operations > and identifying potential mismanagement of reference counts. In this case, > the tool flagged the missing decrement operation as a potential issue, > leading to this patch. > > Fixes: 4035c72dc1ba ("ice: reconfig host after changing MSI-X on VF") > Cc: stable@vger.kernel.org > Signed-off-by: Gui-Dong Han Thanks Gui-Dong Han, I agree with your analysis. However, I wonder if the same resource leak can occur in the unroll label, if the if clause results in a return. It is around line 1444 without your patch applied. vf->first_vector_idx = ice_sriov_get_irqs(pf, vf->num_msix); if (vf->first_vector_idx < 0) return -EINVAL; > --- > drivers/net/ethernet/intel/ice/ice_sriov.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c > index 55ef33208456..eb5030aba9a5 100644 > --- a/drivers/net/ethernet/intel/ice/ice_sriov.c > +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c > @@ -1096,8 +1096,10 @@ int ice_sriov_set_msix_vec_count(struct pci_dev *vf_dev, int msix_vec_count) > return -ENOENT; > > vsi = ice_get_vf_vsi(vf); > - if (!vsi) > + if (!vsi) { > + ice_put_vf(vf); > return -ENOENT; > + } > > prev_msix = vf->num_msix; > prev_queues = vf->num_vf_qs; > -- > 2.25.1 > >