From: Maurizio Lombardi mlombard@redhat.com
[ Upstream commit d8ab68bdb294b09a761e967dad374f2965e1913f ]
The function core_scsi3_decode_spec_i_port(), in its error code path, unconditionally calls core_scsi3_lunacl_undepend_item() passing the dest_se_deve pointer, which may be NULL.
This can lead to a NULL pointer dereference if dest_se_deve remains unset.
SPC-3 PR SPEC_I_PT: Unable to locate dest_tpg Unable to handle kernel paging request at virtual address dfff800000000012 Call trace: core_scsi3_lunacl_undepend_item+0x2c/0xf0 [target_core_mod] (P) core_scsi3_decode_spec_i_port+0x120c/0x1c30 [target_core_mod] core_scsi3_emulate_pro_register+0x6b8/0xcd8 [target_core_mod] target_scsi3_emulate_pr_out+0x56c/0x840 [target_core_mod]
Fix this by adding a NULL check before calling core_scsi3_lunacl_undepend_item()
Signed-off-by: Maurizio Lombardi mlombard@redhat.com Link: https://lore.kernel.org/r/20250612101556.24829-1-mlombard@redhat.com Reviewed-by: Mike Christie michael.christie@oracle.com Reviewed-by: John Meneghini jmeneghi@redhat.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Signed-off-by: Sasha Levin sashal@kernel.org ---
I see now! If we look at the error path, when `core_get_se_deve_from_rtpi()` returns NULL (line 1667), the code jumps to `out_unmap` at line 1675. However, there could be existing entries in the `tid_dest_list` that were already added before this failure.
The commit shows the stack trace mentions "SPC-3 PR SPEC_I_PT: Unable to locate dest_tpg" which actually happens at line 1641, and then jumps to `out_unmap` (line 1644).
In this case, any tidh entries in the list would have `dest_se_deve = NULL` because the failure happens before `dest_se_deve` is even assigned.
**YES**
This commit should be backported to stable kernel trees for the following reasons:
1. **It fixes a real NULL pointer dereference bug**: The commit addresses a kernel crash that occurs when `core_scsi3_lunacl_undepend_item()` is called with a NULL `dest_se_deve` pointer. The function dereferences the pointer at line 1453 (`if (!se_deve->se_lun_acl)`) without checking for NULL, causing a crash.
2. **The bug affects normal error handling paths**: This crash occurs during error handling in the SCSI target subsystem when processing PERSISTENT RESERVE OUT commands with SPEC_I_PT (Specify Initiator Ports). When certain lookups fail (like "Unable to locate dest_tpg"), the error cleanup path can have NULL `dest_se_deve` values in the tid_dest_list.
3. **The fix is minimal and safe**: The fix simply adds a NULL check before calling `core_scsi3_lunacl_undepend_item()`: ```c if (dest_se_deve) core_scsi3_lunacl_undepend_item(dest_se_deve); ``` This is a defensive programming practice that prevents the crash without changing the logic flow.
4. **The fix is confined to error handling**: The change only affects the error cleanup path and doesn't modify the normal operation flow, minimizing the risk of regression.
5. **Similar to other backported fixes**: Looking at the similar commits, commit #3 (qla2xxx NULL pointer fix) and commit #5 (rt5514-spi NULL pointer fix) were both backported and had similar characteristics - they were simple NULL pointer checks that prevented crashes.
6. **Production impact**: NULL pointer dereferences in kernel code lead to kernel panics, which can cause system crashes and data loss. This is particularly important for storage-related code where reliability is critical.
The commit follows the stable tree rules by fixing an important bug (kernel crash) with minimal risk and without introducing new features or architectural changes.
drivers/target/target_core_pr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 4f4ad6af416c8..47fe50b80c229 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -1842,7 +1842,9 @@ core_scsi3_decode_spec_i_port( }
kmem_cache_free(t10_pr_reg_cache, dest_pr_reg); - core_scsi3_lunacl_undepend_item(dest_se_deve); + + if (dest_se_deve) + core_scsi3_lunacl_undepend_item(dest_se_deve);
if (is_local) continue;