patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Alok Tiwari <alok.a.tiwari@oracle.com>,
	Jakub Kicinski <kuba@kernel.org>, Sasha Levin <sashal@kernel.org>,
	ajit.khaparde@broadcom.com, sriharsha.basavapatna@broadcom.com,
	somnath.kotur@broadcom.com, netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 5.10 04/27] emulex/benet: correct command version selection in be_cmd_get_stats()
Date: Tue,  3 Jun 2025 21:05:57 -0400	[thread overview]
Message-ID: <20250604010620.6819-4-sashal@kernel.org> (raw)
In-Reply-To: <20250604010620.6819-1-sashal@kernel.org>

From: Alok Tiwari <alok.a.tiwari@oracle.com>

[ Upstream commit edb888d29748cee674006a52e544925dacc7728e ]

Logic here always sets hdr->version to 2 if it is not a BE3 or Lancer chip,
even if it is BE2. Use 'else if' to prevent multiple assignments, setting
version 0 for BE2, version 1 for BE3 and Lancer, and version 2 for others.
Fixes potential incorrect version setting when BE2_chip and
BE3_chip/lancer_chip checks could both be true.

Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Link: https://patch.msgid.link/20250519141731.691136-1-alok.a.tiwari@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis, let me provide my determination: **YES** This
commit should be backported to stable kernel trees. ## Detailed
Analysis: ### 1. **Nature of the Bug:** The commit fixes a logical error
in command version selection for the `be_cmd_get_stats()` function in
the Emulex be2net driver. The original code had this problematic logic:
```c if (BE2_chip(adapter)) hdr->version = 0; if (BE3_chip(adapter) ||
lancer_chip(adapter)) // <- Missing "else" hdr->version = 1; else
hdr->version = 2; ``` The bug is that the second `if` should be `else
if`. Without the `else if`, if a chip satisfies both `BE2_chip(adapter)`
AND `(BE3_chip(adapter) || lancer_chip(adapter))` conditions, the
version would be incorrectly set multiple times, with the final value
being either 1 or 2 depending on subsequent conditions. ### 2. **Impact
Assessment:** - **User-facing impact:** This bug could cause incorrect
command versions to be sent to the hardware, potentially leading to
statistics retrieval failures or unexpected behavior - **Scope:**
Affects all users of Emulex be2net network adapters where the chip
detection logic could overlap - **Severity:** While not a critical
security issue, it affects driver functionality and could lead to
network statistics reporting failures ### 3. **Code Change Analysis:**
The fix is minimal and surgical: - **Single line change:** Only changes
`if` to `else if` on line 1612 - **No functional additions:** Does not
introduce new features or change APIs - **Low regression risk:** The
change makes the logic more explicit and prevents multiple assignments -
**Clear intent:** The fix aligns with the comment stating "version 1 of
the cmd is not supported only by BE2" ### 4. **Backport Suitability
Criteria:** ✅ **Fixes important bug:** Corrects incorrect hardware
command version selection ✅ **Small and contained:** Single line change
with no side effects ✅ **Low regression risk:** Makes existing logic
more explicit and correct ✅ **No architectural changes:** Pure bug fix
without design modifications ✅ **Subsystem confined:** Only affects
be2net driver statistics functionality ### 5. **Comparison with
Historical Commits:** The provided historical commits were all marked
"NO" for backporting because they were: - Version string updates
(cosmetic changes) - Code simplifications (non-critical improvements) -
Driver version removals (cleanup) In contrast, this commit fixes actual
functional logic that could cause incorrect behavior. ### 6. **Stable
Tree Compliance:** This commit perfectly fits the stable tree criteria:
- Fixes a clear bug in existing code - Has minimal risk of introducing
regressions - Affects a specific driver subsystem - Does not change
user-visible APIs or behavior (except fixing incorrect behavior) The
commit message clearly explains the problem and solution, and the change
is straightforward to verify and test.

 drivers/net/ethernet/emulex/benet/be_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 9812a9a5d033b..d9bceb26f4e5b 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1608,7 +1608,7 @@ int be_cmd_get_stats(struct be_adapter *adapter, struct be_dma_mem *nonemb_cmd)
 	/* version 1 of the cmd is not supported only by BE2 */
 	if (BE2_chip(adapter))
 		hdr->version = 0;
-	if (BE3_chip(adapter) || lancer_chip(adapter))
+	else if (BE3_chip(adapter) || lancer_chip(adapter))
 		hdr->version = 1;
 	else
 		hdr->version = 2;
-- 
2.39.5


  parent reply	other threads:[~2025-06-04  1:06 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  1:05 [PATCH AUTOSEL 5.10 01/27] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.10 02/27] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.10 03/27] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04  1:05 ` Sasha Levin [this message]
2025-06-04  1:05 ` [PATCH AUTOSEL 5.10 05/27] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.10 06/27] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 07/27] i2c: npcm: Add clock toggle recovery Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 08/27] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 09/27] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 10/27] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 11/27] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 12/27] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 13/27] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 14/27] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 15/27] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 16/27] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 17/27] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 18/27] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 19/27] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 20/27] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 21/27] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 22/27] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 23/27] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 24/27] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 25/27] scsi: lpfc: Use memcpy() for BIOS version Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 26/27] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04  1:06 ` [PATCH AUTOSEL 5.10 27/27] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin

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=20250604010620.6819-4-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=ajit.khaparde@broadcom.com \
    --cc=alok.a.tiwari@oracle.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=somnath.kotur@broadcom.com \
    --cc=sriharsha.basavapatna@broadcom.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).