github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-09-11 20:13:02 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

scsi: mpi3mr: Fix use-after-free on tgt_dev->starget during target device refresh/update

Browse Source 
mpi3mr_refresh_tgtdevs() and mpi3mr_devinfochg_evt_bh() read
tgt_dev->starget and immediately pass it to starget_for_each_device()
without holding mrioc->tgtdev_lock. Every writer of this field --
mpi3mr_target_alloc(), mpi3mr_target_destroy(), mpi3mr_slave_destroy()
and mpi3mr_sdev_init() -- correctly serializes access under tgtdev_lock,
but these two read sites do not, which leaves a check-then-use window
against the SCSI core's target teardown path (scsi_remove_target(),
invoked e.g. via a concurrent host reset, sysfs "delete", or SCSI EH
device offlining running independently of the fwevt workqueue).

Sequence observed on production hardware, triggered on the
mpi3mr0_fwevt_wrkr workqueue during a SAS topology change shortly after
a controller reset:

  BUG: kernel NULL pointer dereference, address: 0000000000000058
  RIP: scsi_is_host_device+0x7/0x20
  Call Trace:
   starget_for_each_device+0x34/0x100
   mpi3mr_refresh_tgtdevs+0x152/0x1d0 [mpi3mr]
   mpi3mr_fwevt_bh+0x514/0x6c0 [mpi3mr]
   mpi3mr_fwevt_worker+0x1a/0x50 [mpi3mr]
   process_one_work+0x194/0x380
   worker_thread+0x2fe/0x410

mpi3mr_refresh_tgtdevs() reads tgt_dev->starget as non-NULL, but by the
time starget_for_each_device() dereferences it, a concurrent
mpi3mr_target_destroy() has already cleared tgt_dev->starget under
tgtdev_lock and the SCSI/device core has freed the underlying
scsi_target (and its embedded struct device). The stale pointer is then
walked by dev_to_shost() -> scsi_is_host_device(), producing the
NULL/garbage dereference above.

Fix this by taking mrioc->tgtdev_lock around every read of
tgt_dev->starget, matching the existing writer-side discipline. Since
starget_for_each_device() and mpi3mr_update_sdev() can end up doing
non-atomic work (e.g. queue_limits_commit_update()), the lock cannot be
held across the whole call, so instead pin the target's device with
get_device() while holding the lock, drop the lock, then run
starget_for_each_device() against the pinned reference and put_device()
afterwards. This closes the TOCTOU window instead of merely narrowing
it.

The same unlocked read-and-dereference pattern also exists earlier in
mpi3mr_refresh_tgtdevs()'s first removal-scan loop
(tgt_dev->starget->hostdata); fix it the same way by holding tgtdev_lock
across that check, which is cheap since it only touches plain struct
fields.

Assisted-by: Claude:Sonnet5 [Claude Code]
Signed-off-by: Laurence Oberman <loberman@redhat.com>
Acked-by: Chandrakanth Patil <chandrakanth.patil@broadcom.com>
Link: https://patch.msgid.link/20260831120047.14690-1-loberman@redhat.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
This commit is contained in:
Laurence Oberman 2026-08-31 07:59:17 -04:00 committed by Martin K. Petersen (Oracle)
parent f4825922d2
commit 139f57343b
1 changed files with 35 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
drivers/scsi/mpi3mr/mpi3mr_os.c
View File

@ -1094,10 +1094,13 @@ static void mpi3mr_refresh_tgtdevs(struct mpi3mr_ioc *mrioc)
{
struct mpi3mr_tgt_dev *tgtdev, *tgtdev_next;
struct mpi3mr_stgt_priv_data *tgt_priv;
struct scsi_target *starget;
unsigned long flags;
dprint_reset(mrioc, "refresh target devices: check for removals\n");
list_for_each_entry_safe(tgtdev, tgtdev_next, &mrioc->tgtdev_list,
list) {
spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
if (((tgtdev->dev_handle == MPI3MR_INVALID_DEV_HANDLE) ||
tgtdev->is_hidden) &&
tgtdev->host_exposed && tgtdev->starget &&
@ -1106,6 +1109,7 @@ static void mpi3mr_refresh_tgtdevs(struct mpi3mr_ioc *mrioc)
tgt_priv->dev_removed = 1;
atomic_set(&tgt_priv->block_io, 0);
}
spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
}
list_for_each_entry_safe(tgtdev, tgtdev_next, &mrioc->tgtdev_list,
@ -1127,15 +1131,25 @@ static void mpi3mr_refresh_tgtdevs(struct mpi3mr_ioc *mrioc)
tgtdev = NULL;
list_for_each_entry(tgtdev, &mrioc->tgtdev_list, list) {
if ((tgtdev->dev_handle != MPI3MR_INVALID_DEV_HANDLE) &&
!tgtdev->is_hidden) {
if (!tgtdev->host_exposed)
!tgtdev->is_hidden) {
if (!tgtdev->host_exposed) {
mpi3mr_report_tgtdev_to_host(mrioc,
tgtdev->perst_id);
else if (tgtdev->starget)
starget_for_each_device(tgtdev->starget,
(void *)tgtdev, mpi3mr_update_sdev);
}
tgtdev->perst_id);
continue;
}
spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
starget = tgtdev->starget;
if (starget)
get_device(&starget->dev);
spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
if (starget) {
starget_for_each_device(starget, (void *)tgtdev,
mpi3mr_update_sdev);
put_device(&starget->dev);
}
}
}
dprint_reset(mrioc, "refresh target devices: done\n");
}
/**
@ -1515,6 +1529,8 @@ static void mpi3mr_devinfochg_evt_bh(struct mpi3mr_ioc *mrioc,
struct mpi3_device_page0 *dev_pg0)
{
struct mpi3mr_tgt_dev *tgtdev = NULL;
struct scsi_target *starget;
unsigned long flags;
u16 dev_handle = 0, perst_id = 0;
perst_id = le16_to_cpu(dev_pg0->persistent_id);
@ -1535,9 +1551,18 @@ static void mpi3mr_devinfochg_evt_bh(struct mpi3mr_ioc *mrioc,
mpi3mr_report_tgtdev_to_host(mrioc, perst_id);
if (tgtdev->is_hidden && tgtdev->host_exposed)
mpi3mr_remove_tgtdev_from_host(mrioc, tgtdev);
if (!tgtdev->is_hidden && tgtdev->host_exposed && tgtdev->starget)
starget_for_each_device(tgtdev->starget, (void *)tgtdev,
mpi3mr_update_sdev);
if (!tgtdev->is_hidden && tgtdev->host_exposed) {
spin_lock_irqsave(&mrioc->tgtdev_lock, flags);
starget = tgtdev->starget;
if (starget)
get_device(&starget->dev);
spin_unlock_irqrestore(&mrioc->tgtdev_lock, flags);
if (starget) {
starget_for_each_device(starget, (void *)tgtdev,
mpi3mr_update_sdev);
put_device(&starget->dev);
}
}
out:
if (tgtdev)
mpi3mr_tgtdev_put(tgtdev);