All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Daniel Wagner <wagi@kernel.org>,
	Justin Tee <justin.tee@broadcom.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	Sasha Levin <sashal@kernel.org>,
	james.smart@broadcom.com, dick.kennedy@broadcom.com,
	linux-scsi@vger.kernel.org
Subject: [PATCH AUTOSEL 5.15 29/33] scsi: lpfc: Use memcpy() for BIOS version
Date: Tue,  3 Jun 2025 21:05:20 -0400	[thread overview]
Message-ID: <20250604010524.6091-29-sashal@kernel.org> (raw)
In-Reply-To: <20250604010524.6091-1-sashal@kernel.org>

From: Daniel Wagner <wagi@kernel.org>

[ Upstream commit ae82eaf4aeea060bb736c3e20c0568b67c701d7d ]

The strlcat() with FORTIFY support is triggering a panic because it
thinks the target buffer will overflow although the correct target
buffer size is passed in.

Anyway, instead of memset() with 0 followed by a strlcat(), just use
memcpy() and ensure that the resulting buffer is NULL terminated.

BIOSVersion is only used for the lpfc_printf_log() which expects a
properly terminated string.

Signed-off-by: Daniel Wagner <wagi@kernel.org>
Link: https://lore.kernel.org/r/20250409-fix-lpfc-bios-str-v1-1-05dac9e51e13@kernel.org
Reviewed-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Let me understand the actual problem by examining the function more
carefully. Based on the commit message, this is about a FORTIFY panic
when using strlcat(): **YES** This commit should be backported to stable
kernel trees. Here's my extensive analysis: ## Bug Analysis **Root
Cause:** The commit fixes a kernel panic triggered by FORTIFY_SOURCE
when using `strlcat()`. The FORTIFY infrastructure incorrectly detected
a potential buffer overflow in the BIOS version string handling code.
**The Problem Code:** ```c memset(phba->BIOSVersion, 0,
sizeof(phba->BIOSVersion)); strlcat(phba->BIOSVersion, (char
*)cntl_attr->bios_ver_str, sizeof(phba->BIOSVersion)); ``` **The Fix:**
```c memcpy(phba->BIOSVersion, cntl_attr->bios_ver_str,
sizeof(phba->BIOSVersion)); phba->BIOSVersion[sizeof(phba->BIOSVersion)
- 1] = '\0'; ``` ## Technical Details 1. **Buffer Sizes:** `BIOSVersion`
is a 16-byte character array (`char BIOSVersion[16]` in lpfc.h:1267),
while `bios_ver_str` is a 32-byte array (`uint32_t bios_ver_str[8]` in
lpfc_hw4.h:3798). 2. **FORTIFY Issue:** The `strlcat()` function with
FORTIFY enabled was triggering a false positive, causing kernel panics
when it incorrectly calculated potential overflow conditions. 3. **Safe
Solution:** The fix uses `memcpy()` for the exact buffer size and
explicitly null-terminates the string, which is simpler and avoids the
FORTIFY false positive. ## Backport Criteria Analysis ✅ **Fixes
Important Bug:** This resolves kernel panics in production systems with
FORTIFY enabled ✅ **Small & Contained:** Only 3 lines changed in a
single function ✅ **Minimal Risk:** The change is functionally
equivalent but safer ✅ **No Architectural Changes:** Simple string
handling improvement ✅ **Clear Side Effects:** No behavioral changes
beyond fixing the panic ✅ **Critical Subsystem:** SCSI driver stability
is important for storage ## Comparison with Similar Commits This closely
resembles **Similar Commit #2** (marked YES for backport) which fixed an
overflow build issue with FORTIFY. Both commits: - Address FORTIFY-
related compilation/runtime issues - Make minimal, safe code changes -
Fix real problems affecting users - Have low regression risk ## Risk
Assessment **Low Risk:** The change maintains identical functionality
while eliminating the false positive. Both code paths copy BIOS version
data into a 16-byte buffer and ensure null termination. The `memcpy()`
approach is actually more predictable than `strlcat()` with an empty
target buffer. ## Conclusion This commit fixes a real kernel panic issue
that affects users running kernels with FORTIFY_SOURCE enabled. The fix
is minimal, safe, and follows stable tree guidelines perfectly. It
should definitely be backported to prevent production systems from
experiencing these panics.

 drivers/scsi/lpfc/lpfc_sli.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 68b015bb6d157..fb139e1e35ca3 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -5926,9 +5926,9 @@ lpfc_sli4_get_ctl_attr(struct lpfc_hba *phba)
 	phba->sli4_hba.flash_id = bf_get(lpfc_cntl_attr_flash_id, cntl_attr);
 	phba->sli4_hba.asic_rev = bf_get(lpfc_cntl_attr_asic_rev, cntl_attr);
 
-	memset(phba->BIOSVersion, 0, sizeof(phba->BIOSVersion));
-	strlcat(phba->BIOSVersion, (char *)cntl_attr->bios_ver_str,
+	memcpy(phba->BIOSVersion, cntl_attr->bios_ver_str,
 		sizeof(phba->BIOSVersion));
+	phba->BIOSVersion[sizeof(phba->BIOSVersion) - 1] = '\0';
 
 	lpfc_printf_log(phba, KERN_INFO, LOG_SLI,
 			"3086 lnk_type:%d, lnk_numb:%d, bios_ver:%s, "
-- 
2.39.5


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

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  1:04 [PATCH AUTOSEL 5.15 01/33] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 02/33] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 03/33] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 04/33] emulex/benet: correct command version selection in be_cmd_get_stats() Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 05/33] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 06/33] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 07/33] cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 5.15 08/33] i2c: npcm: Add clock toggle recovery Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 09/33] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 10/33] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 11/33] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 12/33] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 13/33] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 14/33] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 15/33] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 16/33] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 17/33] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 18/33] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 19/33] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 20/33] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 21/33] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04  1:05   ` Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 22/33] libbpf: Add identical pointer detection to btf_dedup_is_equiv() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 23/33] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 24/33] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 25/33] net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 26/33] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 27/33] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 28/33] pinctrl: mcp23s08: Reset all pins to input at probe Sasha Levin
2025-06-04  1:05 ` Sasha Levin [this message]
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 30/33] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04  1:05 ` [Intel-wired-lan] [PATCH AUTOSEL 5.15 31/33] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin
2025-06-04  1:05   ` Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 32/33] bpf, sockmap: Fix data lost during EAGAIN retries Sasha Levin
2025-06-04  1:05 ` [PATCH AUTOSEL 5.15 33/33] octeontx2-pf: Add error log forcn10k_map_unmap_rq_policer() 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=20250604010524.6091-29-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=dick.kennedy@broadcom.com \
    --cc=james.smart@broadcom.com \
    --cc=justin.tee@broadcom.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=wagi@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.