* [PATCH AUTOSEL 6.16-5.4] scsi: bfa: Double-free fix
[not found] <20250805130945.471732-1-sashal@kernel.org>
@ 2025-08-05 13:08 ` Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.12] scsi: lpfc: Ensure HBA_SETUP flag is used only for SLI4 in dev_loss_tmo_callbk Sasha Levin
` (5 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:08 UTC (permalink / raw)
To: patches, stable
Cc: jackysliu, Martin K . Petersen, Sasha Levin, anil.gurumurthy,
sudarsana.kalluru, linux-scsi
From: jackysliu <1972843537@qq.com>
[ Upstream commit add4c4850363d7c1b72e8fce9ccb21fdd2cf5dc9 ]
When the bfad_im_probe() function fails during initialization, the memory
pointed to by bfad->im is freed without setting bfad->im to NULL.
Subsequently, during driver uninstallation, when the state machine enters
the bfad_sm_stopping state and calls the bfad_im_probe_undo() function,
it attempts to free the memory pointed to by bfad->im again, thereby
triggering a double-free vulnerability.
Set bfad->im to NULL if probing fails.
Signed-off-by: jackysliu <1972843537@qq.com>
Link: https://lore.kernel.org/r/tencent_3BB950D6D2D470976F55FC879206DE0B9A09@qq.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit is a clear candidate for backporting to stable kernel trees.
Here's my extensive analysis:
## Bug Analysis
The commit fixes a **critical double-free vulnerability** in the BFA
(Brocade Fibre Channel Adapter) SCSI driver. The issue occurs in the
following scenario:
1. During driver initialization, `bfad_im_probe()` allocates memory for
`bfad->im` (line 700: `im = kzalloc(sizeof(struct bfad_im_s),
GFP_KERNEL)`)
2. The pointer is assigned to `bfad->im` (line 704: `bfad->im = im`)
3. If `bfad_thread_workq()` fails (line 707), the function frees the
memory (line 708: `kfree(im)`) but **fails to set `bfad->im = NULL`**
4. Later, during driver uninstallation, when the state machine enters
`bfad_sm_stopping` state, it calls `bfad_im_probe_undo()` (line 714
in bfad.c)
5. `bfad_im_probe_undo()` checks `if (bfad->im)` (line 720) and since
the pointer wasn't nulled, it attempts to free the already-freed
memory again (line 722: `kfree(bfad->im)`)
## Why This Should Be Backported
1. **Security Impact**: Double-free vulnerabilities can lead to memory
corruption, system crashes, and potentially be exploited for
privilege escalation. This is a real security issue.
2. **Simple Fix**: The fix is minimal - just a single line addition
(`bfad->im = NULL;`) after the `kfree(im)` call. This follows the
exact same pattern already used in `bfad_im_probe_undo()` where after
freeing, the pointer is set to NULL (line 723).
3. **Clear Bug**: The bug is straightforward - the code inconsistently
handles the cleanup path. The fix makes it consistent with the
cleanup function's expectations.
4. **Limited Scope**: The change is confined to error handling in a
single function (`bfad_im_probe()`). It doesn't modify any normal
operation paths or introduce new features.
5. **Low Risk**: Setting a pointer to NULL after freeing is a standard
defensive programming practice with essentially zero risk of
regression.
6. **Affects Users**: Any system using Brocade Fibre Channel adapters
could hit this issue if the driver initialization fails (e.g., due to
memory allocation failure in `bfad_thread_workq()`).
7. **Follows Stable Rules**: The commit:
- Fixes a real bug (double-free vulnerability)
- Is small and contained (1 line change)
- Has clear causality
- Doesn't introduce new features
- Has minimal risk of causing regressions
The commit follows the Linux stable kernel rules perfectly - it's a
small, obvious fix for a real bug that could affect users in production
environments.
drivers/scsi/bfa/bfad_im.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/bfa/bfad_im.c b/drivers/scsi/bfa/bfad_im.c
index a719a18f0fbc..f56e008ee52b 100644
--- a/drivers/scsi/bfa/bfad_im.c
+++ b/drivers/scsi/bfa/bfad_im.c
@@ -706,6 +706,7 @@ bfad_im_probe(struct bfad_s *bfad)
if (bfad_thread_workq(bfad) != BFA_STATUS_OK) {
kfree(im);
+ bfad->im = NULL;
return BFA_STATUS_FAILED;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-6.12] scsi: lpfc: Ensure HBA_SETUP flag is used only for SLI4 in dev_loss_tmo_callbk
[not found] <20250805130945.471732-1-sashal@kernel.org>
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] scsi: bfa: Double-free fix Sasha Levin
@ 2025-08-05 13:08 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: lpfc: Check for hdwq null ptr when cleaning up lpfc_vport structure Sasha Levin
` (4 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:08 UTC (permalink / raw)
To: patches, stable
Cc: Justin Tee, Martin K . Petersen, Sasha Levin, james.smart,
dick.kennedy, linux-scsi
From: Justin Tee <justin.tee@broadcom.com>
[ Upstream commit 1cced5779e7a3ff7ec025fc47c76a7bd3bb38877 ]
For SLI3, the HBA_SETUP flag is never set so the lpfc_dev_loss_tmo_callbk
always early returns. Add a phba->sli_rev check for SLI4 mode so that
the SLI3 path can flow through the original dev_loss_tmo worker thread
design to lpfc_dev_loss_tmo_handler instead of early return.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20250618192138.124116-9-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough context. Let me analyze the commit thoroughly.
**Backport Status: YES**
This commit fixes a significant bug in the lpfc SCSI driver that causes
SLI3 mode HBAs to completely bypass dev_loss_tmo handling, which is a
critical issue for device loss timeout processing.
## Analysis:
### 1. **Bug Description:**
The commit fixes a bug where `lpfc_dev_loss_tmo_callbk()` always returns
early for SLI3 HBAs because the `HBA_SETUP` flag is never set for SLI3
mode. The code at line 186 checks:
```c
if (test_bit(FC_UNLOADING, &vport->load_flag) ||
!test_bit(HBA_SETUP, &phba->hba_flag))
```
### 2. **Root Cause:**
- The `HBA_SETUP` flag is only set in `lpfc_sli4_hba_setup()` function
(drivers/scsi/lpfc/lpfc_sli.c:8850)
- For SLI3, the initialization function is `lpfc_sli_hba_setup()` which
never sets this flag
- This check was incorrectly added in commit 2be1d4f11944c (July 2024)
without considering SLI3 mode
### 3. **Impact:**
- **SLI3 HBAs completely lose dev_loss_tmo functionality**: The early
return prevents the worker thread event from being scheduled (lines
277-289), which means `lpfc_dev_loss_tmo_handler()` is never called
- **Device removal handling is broken**: Without proper dev_loss_tmo
processing, devices that disappear from the fabric won't be properly
cleaned up
- **Memory leaks and stale references**: Node references won't be
properly released, potentially causing memory leaks
### 4. **The Fix:**
The fix adds a proper SLI revision check:
```c
if (test_bit(FC_UNLOADING, &vport->load_flag) ||
(phba->sli_rev == LPFC_SLI_REV4 &&
!test_bit(HBA_SETUP, &phba->hba_flag)))
```
This ensures the HBA_SETUP check only applies to SLI4, allowing SLI3 to
flow through to the original worker thread design.
### 5. **Stable Backport Criteria:**
✓ **Fixes a real bug**: Completely broken dev_loss_tmo for SLI3 HBAs
✓ **Small and contained**: Single-line logic fix with minimal risk
✓ **No architectural changes**: Simply corrects a logic error
✓ **Critical functionality**: dev_loss_tmo is essential for proper SCSI
device management
✓ **Regression fix**: This fixes a regression introduced by commit
2be1d4f11944c
### 6. **Risk Assessment:**
- **Very low risk**: The change only adds a SLI revision check before an
existing condition
- **Well-tested code path**: Restores SLI3 to use the original, well-
tested dev_loss_tmo path
- **No new functionality**: Simply fixes broken existing functionality
This is a textbook example of a stable backport candidate - it fixes a
serious regression that completely breaks essential functionality for
SLI3 HBAs with a minimal, safe change.
drivers/scsi/lpfc/lpfc_hbadisc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index b88e54a7e65c..3962f07c9140 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -183,7 +183,8 @@ lpfc_dev_loss_tmo_callbk(struct fc_rport *rport)
/* Don't schedule a worker thread event if the vport is going down. */
if (test_bit(FC_UNLOADING, &vport->load_flag) ||
- !test_bit(HBA_SETUP, &phba->hba_flag)) {
+ (phba->sli_rev == LPFC_SLI_REV4 &&
+ !test_bit(HBA_SETUP, &phba->hba_flag))) {
spin_lock_irqsave(&ndlp->lock, iflags);
ndlp->rport = NULL;
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-5.4] scsi: lpfc: Check for hdwq null ptr when cleaning up lpfc_vport structure
[not found] <20250805130945.471732-1-sashal@kernel.org>
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-5.4] scsi: bfa: Double-free fix Sasha Levin
2025-08-05 13:08 ` [PATCH AUTOSEL 6.16-6.12] scsi: lpfc: Ensure HBA_SETUP flag is used only for SLI4 in dev_loss_tmo_callbk Sasha Levin
@ 2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] scsi: pm80xx: Free allocated tags after failure Sasha Levin
` (3 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:09 UTC (permalink / raw)
To: patches, stable
Cc: Justin Tee, Martin K . Petersen, Sasha Levin, james.smart,
dick.kennedy, linux-scsi
From: Justin Tee <justin.tee@broadcom.com>
[ Upstream commit 6698796282e828733cde3329c887b4ae9e5545e9 ]
If a call to lpfc_sli4_read_rev() from lpfc_sli4_hba_setup() fails, the
resultant cleanup routine lpfc_sli4_vport_delete_fcp_xri_aborted() may
occur before sli4_hba.hdwqs are allocated. This may result in a null
pointer dereference when attempting to take the abts_io_buf_list_lock for
the first hardware queue. Fix by adding a null ptr check on
phba->sli4_hba.hdwq and early return because this situation means there
must have been an error during port initialization.
Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20250618192138.124116-4-justintee8345@gmail.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
So, the issue is:
1. During driver initialization in `lpfc_sli4_pci_probe()` (around line
14868), `lpfc_sli4_hba_setup()` is called
2. Inside `lpfc_sli4_hba_setup()` at line 8517, `lpfc_sli4_read_rev()`
is called
3. If `lpfc_sli4_read_rev()` fails, we goto `out_free_mbox` which
returns error
4. The queue creation (`lpfc_sli4_queue_create()` at line 8811) that
allocates `hdwq` is never reached
5. When the error is returned to `lpfc_sli4_pci_probe()`, it goes to
`out_free_sysfs_attr`
6. This error path calls `lpfc_destroy_shost()` -> `destroy_port()` ->
`lpfc_cleanup()` -> `lpfc_cleanup_vports_rrqs()` ->
`lpfc_sli4_vport_delete_fcp_xri_aborted()`
7. `lpfc_sli4_vport_delete_fcp_xri_aborted()` tries to access
`phba->sli4_hba.hdwq[idx]` which is NULL, causing a null pointer
dereference
## Backport Analysis
**YES**
This commit should be backported to stable kernel trees for the
following reasons:
1. **Critical Bug Fix**: The commit fixes a NULL pointer dereference
that can cause a kernel crash. This is a serious bug that affects
system stability when the lpfc driver fails to initialize properly.
2. **Clear Error Path Issue**: The bug occurs in a well-defined error
handling path - when `lpfc_sli4_read_rev()` fails during HBA setup.
This is not a theoretical issue but can happen in real-world
scenarios when hardware initialization fails.
3. **Small and Contained Fix**: The fix is minimal - just adding a NULL
check with early return (4 lines of code). This follows the stable
kernel criteria of being a small, focused fix that addresses a
specific issue.
4. **No Architectural Changes**: The fix doesn't introduce any new
features or change the driver architecture. It simply adds defensive
programming to prevent accessing unallocated memory.
5. **Low Risk of Regression**: The added check `if
(!phba->sli4_hba.hdwq) return;` is safe - if hdwq is NULL, the
function cannot proceed anyway. The early return prevents the crash
without affecting normal operation when hdwq is properly allocated.
6. **Matches Stable Rules**: According to stable kernel rules, fixes for
kernel crashes and oopses are prime candidates for backporting. This
commit specifically prevents a NULL pointer dereference that would
result in a kernel oops.
7. **Driver Initialization Robustness**: The issue affects error
handling during driver initialization, which is a critical path.
Failures during initialization should be handled gracefully without
causing system crashes.
The commit message clearly describes the problem scenario and the
solution, making it easy for stable maintainers to understand the
importance of this fix. The fact that it adds a defensive check before
dereferencing a pointer that may not be allocated in error paths is
exactly the type of robustness improvement that stable kernels should
receive.
drivers/scsi/lpfc/lpfc_scsi.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index 8acb744febcd..31a9f142bcb9 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -390,6 +390,10 @@ lpfc_sli4_vport_delete_fcp_xri_aborted(struct lpfc_vport *vport)
if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_FCP))
return;
+ /* may be called before queues established if hba_setup fails */
+ if (!phba->sli4_hba.hdwq)
+ return;
+
spin_lock_irqsave(&phba->hbalock, iflag);
for (idx = 0; idx < phba->cfg_hdw_queue; idx++) {
qp = &phba->sli4_hba.hdwq[idx];
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-6.15] scsi: pm80xx: Free allocated tags after failure
[not found] <20250805130945.471732-1-sashal@kernel.org>
` (2 preceding siblings ...)
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: lpfc: Check for hdwq null ptr when cleaning up lpfc_vport structure Sasha Levin
@ 2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] scsi: mpi3mr: Correctly handle ATA device errors Sasha Levin
` (2 subsequent siblings)
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:09 UTC (permalink / raw)
To: patches, stable
Cc: Francisco Gutierrez, Jack Wang, Martin K . Petersen, Sasha Levin,
jinpu.wang, linux-scsi
From: Francisco Gutierrez <frankramirez@google.com>
[ Upstream commit 258a0a19621793b811356fc9d1849f950629d669 ]
This change frees resources after an error is detected.
Signed-off-by: Francisco Gutierrez <frankramirez@google.com>
Link: https://lore.kernel.org/r/20250617210443.989058-1-frankramirez@google.com
Acked-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the
following reasons:
## Bug Fix Analysis
1. **Clear Resource Leak Bug**: The commit fixes a resource leak where
tags allocated via `pm8001_tag_alloc()` are not freed when
`pm8001_mpi_build_cmd()` fails. Looking at the code:
- In `pm80xx_chip_phy_start_req()` at line 4652-4656, a tag is
allocated
- If `pm8001_mpi_build_cmd()` fails (returns error), the function
returns directly without freeing the allocated tag
- The same pattern exists in `pm80xx_chip_phy_stop_req()`
2. **Actual Runtime Impact**: The `pm8001_mpi_build_cmd()` function can
fail with `-ENOMEM` when no free MPI buffers are available (as shown
in the function implementation). This is a realistic failure scenario
under memory pressure or high I/O load, making this a real-world bug.
3. **Resource Exhaustion Risk**: The driver uses a limited tag pool
(PM8001_RESERVE_SLOT tags). Each leaked tag reduces the available
pool, potentially leading to:
- Tag exhaustion over time
- Inability to issue new PHY start/stop commands
- Degraded SCSI controller functionality
4. **Consistent Pattern Fix**: The codebase already has established
patterns for properly freeing tags on error paths, as evidenced by:
- Multiple existing instances where `pm8001_tag_free()` is called
after `pm8001_mpi_build_cmd()` failures
- Previous similar fix in commit c13e73317458 for tag leaks in
`OPC_INB_SET_CONTROLLER_CONFIG` command
5. **Small and Contained Fix**: The changes are minimal (4 lines added
in total), localized to two functions, and follow existing error
handling patterns in the driver. This minimizes regression risk.
6. **No New Features or Architecture Changes**: The commit purely fixes
a resource leak without introducing new functionality or changing
driver behavior.
7. **Maintainer Acknowledgment**: The fix is acknowledged by the
subsystem maintainer (Jack Wang), indicating it's a legitimate issue
that needed addressing.
The fix aligns perfectly with stable kernel criteria - it's a clear bug
fix for a resource leak that can impact system stability over time, with
minimal code changes and low regression risk.
drivers/scsi/pm8001/pm80xx_hwi.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 5b373c53c036..c4074f062d93 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4677,8 +4677,12 @@ pm80xx_chip_phy_start_req(struct pm8001_hba_info *pm8001_ha, u8 phy_id)
&pm8001_ha->phy[phy_id].dev_sas_addr, SAS_ADDR_SIZE);
payload.sas_identify.phy_id = phy_id;
- return pm8001_mpi_build_cmd(pm8001_ha, 0, opcode, &payload,
+ ret = pm8001_mpi_build_cmd(pm8001_ha, 0, opcode, &payload,
sizeof(payload), 0);
+ if (ret < 0)
+ pm8001_tag_free(pm8001_ha, tag);
+
+ return ret;
}
/**
@@ -4704,8 +4708,12 @@ static int pm80xx_chip_phy_stop_req(struct pm8001_hba_info *pm8001_ha,
payload.tag = cpu_to_le32(tag);
payload.phy_id = cpu_to_le32(phy_id);
- return pm8001_mpi_build_cmd(pm8001_ha, 0, opcode, &payload,
+ ret = pm8001_mpi_build_cmd(pm8001_ha, 0, opcode, &payload,
sizeof(payload), 0);
+ if (ret < 0)
+ pm8001_tag_free(pm8001_ha, tag);
+
+ return ret;
}
/*
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-6.1] scsi: mpi3mr: Correctly handle ATA device errors
[not found] <20250805130945.471732-1-sashal@kernel.org>
` (3 preceding siblings ...)
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.15] scsi: pm80xx: Free allocated tags after failure Sasha Levin
@ 2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: mpt3sas: " Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: libiscsi: Initialize iscsi_conn->dd_data only if memory is allocated Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:09 UTC (permalink / raw)
To: patches, stable
Cc: Damien Le Moal, Yafang Shao, Martin K . Petersen, Sasha Levin,
sathya.prakash, kashyap.desai, sumit.saxena, sreekanth.reddy,
mpi3mr-linuxdrv.pdl, linux-scsi
From: Damien Le Moal <dlemoal@kernel.org>
[ Upstream commit 04caad5a7ba86e830d04750417a15bad8ac2613c ]
With the ATA error model, an NCQ command failure always triggers an abort
(termination) of all NCQ commands queued on the device. In such case, the
SAT or the host must handle the failed command according to the command
sense data and immediately retry all other NCQ commands that were aborted
due to the failed NCQ command.
For SAS HBAs controlled by the mpi3mr driver, NCQ command aborts are not
handled by the HBA SAT and sent back to the host, with an ioc log
information equal to 0x31080000 (IOC_LOGINFO_PREFIX_PL with the PL code
PL_LOGINFO_CODE_SATA_NCQ_FAIL_ALL_CMDS_AFTR_ERR). The function
mpi3mr_process_op_reply_desc() always forces a retry of commands
terminated with the status MPI3_IOCSTATUS_SCSI_IOC_TERMINATED using the
SCSI result DID_SOFT_ERROR, regardless of the ioc_loginfo for the
command. This correctly forces the retry of collateral NCQ abort
commands, but with the retry counter for the command being incremented.
If a command to an ATA device is subject to too many retries due to other
NCQ commands failing (e.g. read commands trying to access unreadable
sectors), the collateral NCQ abort commands may be terminated with an
error as they run out of retries. This violates the SAT specification and
causes hard-to-debug command errors.
Solve this issue by modifying the handling of the
MPI3_IOCSTATUS_SCSI_IOC_TERMINATED status to check if a command is for an
ATA device and if the command ioc_loginfo indicates an NCQ collateral
abort. If that is the case, force the command retry using the SCSI result
DID_IMM_RETRY to avoid incrementing the command retry count.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20250606052747.742998-2-dlemoal@kernel.org
Tested-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should be backported to stable kernel trees. Here's my
extensive analysis:
## Critical Bug Fix for ATA NCQ Command Handling
This commit fixes a **data integrity and reliability issue** affecting
ATA devices connected through mpi3mr SAS HBAs when NCQ (Native Command
Queuing) commands fail.
### The Bug Being Fixed
The code change addresses a specific failure mode where:
1. **NCQ Command Collateral Aborts**: When one NCQ command fails on an
ATA device, the SATA specification requires ALL other NCQ commands to
be aborted and retried
2. **Current Broken Behavior**: The driver was treating these collateral
aborts with `DID_SOFT_ERROR`, which increments retry counters
3. **Cascading Failures**: Innocent NCQ commands that were collaterally
aborted could exhaust their retry limits and fail permanently, even
though they had no actual errors
### Code Analysis
The key change in `mpi3mr_process_op_reply_desc()` at line 3432-3433:
**Before:**
```c
case MPI3_IOCSTATUS_SCSI_IOC_TERMINATED:
scmd->result = DID_SOFT_ERROR << 16;
break;
```
**After:**
```c
case MPI3_IOCSTATUS_SCSI_IOC_TERMINATED:
if (ioc_loginfo == IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR) {
WARN_ON_ONCE(xfer_count != 0);
scmd->result = DID_IMM_RETRY << 16;
} else {
scmd->result = DID_SOFT_ERROR << 16;
}
break;
```
The new code:
- Checks for the specific log info code `0x31080000` that indicates an
NCQ collateral abort
- Uses `DID_IMM_RETRY` instead of `DID_SOFT_ERROR` for these specific
cases
- `DID_IMM_RETRY` retries the command immediately WITHOUT incrementing
the retry counter
- Adds a `WARN_ON_ONCE` sanity check that no data was transferred
### Why This Qualifies for Stable
1. **Fixes Real User-Impacting Bug**: This fixes command failures that
shouldn't happen - healthy I/O operations failing due to retry
exhaustion when they were only collateral damage
2. **Data Integrity Risk**: Commands that should succeed but fail due to
retry exhaustion can lead to:
- Application errors
- Filesystem corruption risks
- Database consistency issues
3. **Minimal and Contained Fix**:
- Only 11 lines of actual code change
- Confined to one specific error path
- Only affects ATA devices with specific error condition
- Maintains existing behavior for all other cases
4. **Follows Stable Rules**:
- Fixes a bug that users can hit
- Small, targeted change
- No new features added
- No architectural changes
- Low regression risk
5. **Clear Technical Merit**:
- The commit message explicitly references SAT specification
compliance
- Has a `Tested-by` tag from another developer
- The logic is sound - NCQ collateral aborts are not real failures
6. **Hardware Compatibility**: This affects common SATA devices
connected to enterprise SAS controllers, a configuration used in many
production systems
The change is conservative, well-tested, and fixes a specification
compliance issue that can cause spurious command failures in production
environments.
drivers/scsi/mpi3mr/mpi3mr_os.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index ce444efd859e..87983ea4e06e 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -49,6 +49,13 @@ static void mpi3mr_send_event_ack(struct mpi3mr_ioc *mrioc, u8 event,
#define MPI3_EVENT_WAIT_FOR_DEVICES_TO_REFRESH (0xFFFE)
+/*
+ * SAS Log info code for a NCQ collateral abort after an NCQ error:
+ * IOC_LOGINFO_PREFIX_PL | PL_LOGINFO_CODE_SATA_NCQ_FAIL_ALL_CMDS_AFTR_ERR
+ * See: drivers/message/fusion/lsi/mpi_log_sas.h
+ */
+#define IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR 0x31080000
+
/**
* mpi3mr_host_tag_for_scmd - Get host tag for a scmd
* @mrioc: Adapter instance reference
@@ -3430,7 +3437,18 @@ void mpi3mr_process_op_reply_desc(struct mpi3mr_ioc *mrioc,
scmd->result = DID_NO_CONNECT << 16;
break;
case MPI3_IOCSTATUS_SCSI_IOC_TERMINATED:
- scmd->result = DID_SOFT_ERROR << 16;
+ if (ioc_loginfo == IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR) {
+ /*
+ * This is a ATA NCQ command aborted due to another NCQ
+ * command failure. We must retry this command
+ * immediately but without incrementing its retry
+ * counter.
+ */
+ WARN_ON_ONCE(xfer_count != 0);
+ scmd->result = DID_IMM_RETRY << 16;
+ } else {
+ scmd->result = DID_SOFT_ERROR << 16;
+ }
break;
case MPI3_IOCSTATUS_SCSI_TASK_TERMINATED:
case MPI3_IOCSTATUS_SCSI_EXT_TERMINATED:
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-5.4] scsi: mpt3sas: Correctly handle ATA device errors
[not found] <20250805130945.471732-1-sashal@kernel.org>
` (4 preceding siblings ...)
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-6.1] scsi: mpi3mr: Correctly handle ATA device errors Sasha Levin
@ 2025-08-05 13:09 ` Sasha Levin
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: libiscsi: Initialize iscsi_conn->dd_data only if memory is allocated Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:09 UTC (permalink / raw)
To: patches, stable
Cc: Damien Le Moal, Yafang Shao, Martin K . Petersen, Sasha Levin,
sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
MPT-FusionLinux.pdl, linux-scsi
From: Damien Le Moal <dlemoal@kernel.org>
[ Upstream commit 15592a11d5a5c8411ac8494ec49736b658f6fbff ]
With the ATA error model, an NCQ command failure always triggers an abort
(termination) of all NCQ commands queued on the device. In such case, the
SAT or the host must handle the failed command according to the command
sense data and immediately retry all other NCQ commands that were aborted
due to the failed NCQ command.
For SAS HBAs controlled by the mpt3sas driver, NCQ command aborts are not
handled by the HBA SAT and sent back to the host, with an ioc log
information equal to 0x31080000 (IOC_LOGINFO_PREFIX_PL with the PL code
PL_LOGINFO_CODE_SATA_NCQ_FAIL_ALL_CMDS_AFTR_ERR). The function
_scsih_io_done() always forces a retry of commands terminated with the
status MPI2_IOCSTATUS_SCSI_IOC_TERMINATED using the SCSI result
DID_SOFT_ERROR, regardless of the log_info for the command. This
correctly forces the retry of collateral NCQ abort commands, but with the
retry counter for the command being incremented. If a command to an ATA
device is subject to too many retries due to other NCQ commands failing
(e.g. read commands trying to access unreadable sectors), the collateral
NCQ abort commands may be terminated with an error as they run out of
retries. This violates the SAT specification and causes hard-to-debug
command errors.
Solve this issue by modifying the handling of the
MPI2_IOCSTATUS_SCSI_IOC_TERMINATED status to check if a command is for an
ATA device and if the command loginfo indicates an NCQ collateral
abort. If that is the case, force the command retry using the SCSI result
DID_IMM_RETRY to avoid incrementing the command retry count.
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://lore.kernel.org/r/20250606052747.742998-3-dlemoal@kernel.org
Tested-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Based on my analysis of the commit and examination of the kernel
repository, here is my assessment:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the
following reasons:
## Bug Fix Analysis
1. **Fixes a Significant Bug**: The commit fixes a violation of the SAT
(SCSI ATA Translation) specification that causes incorrect error
handling for NCQ (Native Command Queuing) commands on SATA devices.
When one NCQ command fails, all other NCQ commands get aborted as
collateral damage, which is expected ATA behavior. However, the
driver was incorrectly incrementing retry counters for these
collateral aborts, eventually causing them to fail with errors.
2. **Real User Impact**: The commit message explicitly states this
causes "hard-to-debug command errors" and includes a "Tested-by" tag
from another developer (Yafang Shao), indicating this was a real
problem affecting users in production.
3. **Small and Contained Fix**: The change is minimal - it only adds:
- A new constant definition `IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR`
(0x31080000)
- A small conditional block (lines 5817-5829 in the new code) that
checks for this specific log_info value and uses `DID_IMM_RETRY`
instead of `DID_SOFT_ERROR`
4. **Low Risk of Regression**:
- The fix is highly targeted - it only affects the specific case
where `log_info == IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR`
- It uses an existing SCSI mechanism (`DID_IMM_RETRY`) that's already
well-established in the kernel (used in 30+ places across SCSI
drivers)
- The change preserves all existing behavior for other error
conditions
5. **Fixes Specification Compliance**: The bug violates the SAT
specification, which is important for interoperability and correct
SATA device operation through SAS HBAs.
## Code Analysis
The key change is in the `MPI2_IOCSTATUS_SCSI_IOC_TERMINATED` case
handling:
```c
+if (log_info == IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR) {
+ /*
+ * This is a ATA NCQ command aborted due to another NCQ
+ * command failure. We must retry this command
+ * immediately but without incrementing its retry
+ * counter.
+ */
+ WARN_ON_ONCE(xfer_cnt != 0);
+ scmd->result = DID_IMM_RETRY << 16;
+ break;
+}
```
This specifically handles the NCQ collateral abort case (identified by
the log_info value 0x31080000) by using `DID_IMM_RETRY` which retries
the command without incrementing the retry counter, instead of the
default `DID_SOFT_ERROR` which would increment the counter.
## Stable Tree Criteria Met
- ✓ Fixes a real bug that affects users
- ✓ No new features added
- ✓ Minimal architectural changes
- ✓ Low risk of regression
- ✓ Confined to a specific subsystem (mpt3sas driver)
- ✓ Clear problem and solution
The commit meets all the criteria for stable tree backporting as it's a
targeted bug fix that resolves a specification compliance issue with
minimal risk.
drivers/scsi/mpt3sas/mpt3sas_scsih.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 508861e88d9f..d7d8244dfedc 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -195,6 +195,14 @@ struct sense_info {
#define MPT3SAS_PORT_ENABLE_COMPLETE (0xFFFD)
#define MPT3SAS_ABRT_TASK_SET (0xFFFE)
#define MPT3SAS_REMOVE_UNRESPONDING_DEVICES (0xFFFF)
+
+/*
+ * SAS Log info code for a NCQ collateral abort after an NCQ error:
+ * IOC_LOGINFO_PREFIX_PL | PL_LOGINFO_CODE_SATA_NCQ_FAIL_ALL_CMDS_AFTR_ERR
+ * See: drivers/message/fusion/lsi/mpi_log_sas.h
+ */
+#define IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR 0x31080000
+
/**
* struct fw_event_work - firmware event struct
* @list: link list framework
@@ -5814,6 +5822,17 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
scmd->result = DID_TRANSPORT_DISRUPTED << 16;
goto out;
}
+ if (log_info == IOC_LOGINFO_SATA_NCQ_FAIL_AFTER_ERR) {
+ /*
+ * This is a ATA NCQ command aborted due to another NCQ
+ * command failure. We must retry this command
+ * immediately but without incrementing its retry
+ * counter.
+ */
+ WARN_ON_ONCE(xfer_cnt != 0);
+ scmd->result = DID_IMM_RETRY << 16;
+ break;
+ }
if (log_info == 0x31110630) {
if (scmd->retries > 2) {
scmd->result = DID_NO_CONNECT << 16;
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH AUTOSEL 6.16-5.4] scsi: libiscsi: Initialize iscsi_conn->dd_data only if memory is allocated
[not found] <20250805130945.471732-1-sashal@kernel.org>
` (5 preceding siblings ...)
2025-08-05 13:09 ` [PATCH AUTOSEL 6.16-5.4] scsi: mpt3sas: " Sasha Levin
@ 2025-08-05 13:09 ` Sasha Levin
6 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2025-08-05 13:09 UTC (permalink / raw)
To: patches, stable
Cc: Showrya M N, Potnuri Bharat Teja, Chris Leech,
Martin K . Petersen, Sasha Levin, lduncan, michael.christie,
James.Bottomley, open-iscsi, linux-scsi
From: Showrya M N <showrya@chelsio.com>
[ Upstream commit 3ea3a256ed81f95ab0f3281a0e234b01a9cae605 ]
In case of an ib_fast_reg_mr allocation failure during iSER setup, the
machine hits a panic because iscsi_conn->dd_data is initialized
unconditionally, even when no memory is allocated (dd_size == 0). This
leads invalid pointer dereference during connection teardown.
Fix by setting iscsi_conn->dd_data only if memory is actually allocated.
Panic trace:
------------
iser: iser_create_fastreg_desc: Failed to allocate ib_fast_reg_mr err=-12
iser: iser_alloc_rx_descriptors: failed allocating rx descriptors / data buffers
BUG: unable to handle page fault for address: fffffffffffffff8
RIP: 0010:swake_up_locked.part.5+0xa/0x40
Call Trace:
complete+0x31/0x40
iscsi_iser_conn_stop+0x88/0xb0 [ib_iser]
iscsi_stop_conn+0x66/0xc0 [scsi_transport_iscsi]
iscsi_if_stop_conn+0x14a/0x150 [scsi_transport_iscsi]
iscsi_if_rx+0x1135/0x1834 [scsi_transport_iscsi]
? netlink_lookup+0x12f/0x1b0
? netlink_deliver_tap+0x2c/0x200
netlink_unicast+0x1ab/0x280
netlink_sendmsg+0x257/0x4f0
? _copy_from_user+0x29/0x60
sock_sendmsg+0x5f/0x70
Signed-off-by: Showrya M N <showrya@chelsio.com>
Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
Link: https://lore.kernel.org/r/20250627112329.19763-1-showrya@chelsio.com
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I see the pattern clearly! When `iscsi_alloc_conn` is called with
`dd_size = 0`:
- It allocates only `sizeof(*conn)` (no extra space)
- It sets `conn->dd_data = NULL` (because dd_size is 0, the condition
`if (dd_size)` is false)
But in the original code in `iscsi_conn_setup()`, it was unconditionally
setting:
```c
conn->dd_data = cls_conn->dd_data + sizeof(*conn);
```
This would set `conn->dd_data` to point past the allocated memory when
`dd_size = 0`, leading to the panic when the iSER driver tries to
dereference it in `iscsi_iser_conn_stop()`.
**Backport Status: YES**
This commit should definitely be backported to stable kernel trees for
the following reasons:
1. **Critical Bug Fix**: This fixes a kernel panic/crash that occurs
during iSCSI/iSER connection teardown when memory allocation fails.
The panic trace shows a page fault at address `fffffffffffffff8`,
which is clearly an invalid pointer dereference.
2. **Real-World Impact**: The bug triggers when `ib_fast_reg_mr`
allocation fails during iSER setup, which can happen under memory
pressure. This leads to a system crash during error recovery, turning
a recoverable allocation failure into a fatal kernel panic.
3. **Small and Contained Fix**: The fix is minimal - just adding a
single conditional check:
```c
if (dd_size)
conn->dd_data = cls_conn->dd_data + sizeof(*conn);
```
This only affects the initialization path when `dd_size = 0`.
4. **No Architectural Changes**: This is a straightforward bug fix that
doesn't introduce new features or change any architecture. It simply
prevents setting an invalid pointer when no driver-specific data is
allocated.
5. **Clear Root Cause**: The issue is well-understood: when drivers like
iSER pass `dd_size = 0` to `iscsi_conn_setup()`, the code was
unconditionally calculating a pointer past the allocated memory.
During teardown, accessing this invalid pointer causes the panic.
6. **Minimal Risk**: The change only affects the case where `dd_size =
0`, which is specifically the iSER driver's usage pattern. Other
drivers that pass non-zero `dd_size` values are unaffected.
7. **Follows Stable Rules**: This meets all stable kernel criteria:
- Fixes a real bug that users can hit
- Fix is already upstream (has proper sign-offs and review)
- Small change (2 lines)
- Obviously correct
- Doesn't add new features
The commit prevents a kernel panic during error handling in the
iSCSI/iSER subsystem, making it an important stability fix that should
be backported to all maintained stable kernels that include the iSER
driver.
drivers/scsi/libiscsi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 392d57e054db..c9f410c50978 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -3185,7 +3185,8 @@ iscsi_conn_setup(struct iscsi_cls_session *cls_session, int dd_size,
return NULL;
conn = cls_conn->dd_data;
- conn->dd_data = cls_conn->dd_data + sizeof(*conn);
+ if (dd_size)
+ conn->dd_data = cls_conn->dd_data + sizeof(*conn);
conn->session = session;
conn->cls_conn = cls_conn;
conn->c_stage = ISCSI_CONN_INITIAL_STAGE;
--
2.39.5
^ permalink raw reply related [flat|nested] 7+ messages in thread