scsi: snic: Fix scsi host leak on workqueue allocation failure

PATCH Applied Chen Changcheng 4 participants 5 messages Jul 27 – Aug 26

Subsystems (2)

SCSI Cisco SCSI HBA driver

lore

Share & tools

lore

Chen Changcheng

author

2026-07-27 07:34 UTC #0 lore raw

PATCH 1/1 scsi: snic: Fix scsi host leak on workqueue allocation failure

In snic_add_host(), if scsi_add_host() succeeds but alloc_ordered_workqueue() fails, the function returns -ENOMEM with shost->work_q left as NULL. The caller's error path then calls snic_del_host(), which returns early when !shost->work_q without calling scsi_remove_host(). The Scsi_Host remains registered in sysfs as a zombie device even after the probe has failed. This causes:

- The leaked host remains visible in /sys/class/scsi_host/ after probe failure, with state "running".
- Subsequent SCSI host numbering is permanently shifted (the leaked host ID from ida_alloc() is never reclaimed).
- Memory leak: the Scsi_Host allocation can never be freed because device_add() took a reference that can only be released by device_del() inside scsi_remove_host().

Fix by adding scsi_remove_host() in the workqueue allocation failure path inside snic_add_host(), undoing the successful scsi_add_host() before returning the error. This is cleaner than modifying snic_del_host() because snic_del_host() is called from a shared error label that also serves paths where snic_add_host() was never invoked.

Reproducer (requires no real SNIC hardware):
- Build CONFIG_SCSI_SNIC=y (built-in)
- Add snic.test_mode=1 snic.inject_wq_fail=1 to kernel cmdline
- Boot with a PCI device matching the snic driver (e.g. QEMU edu device, PCI ID 0x1234:0x11e8, temporarily added to the driver's PCI ID table)

Before the fix:

  # /sys/class/scsi_host/ contains a zombie host0:
  $ cat /sys/class/scsi_host/host0/proc_name
  snic_scsi
  $ cat /sys/class/scsi_host/host0/state
  running
  # ata_piix gets host1, host2 (host0 stuck):
  scsi host1: ata_piix
  scsi host2: ata_piix

After the fix:

  # host0 is properly freed and reused by ata_piix:
  scsi host0: ata_piix
  scsi host1: ata_piix
  # No zombie host in /sys/class/scsi_host/

Signed-off-by Chen Changcheng <[email protected]>

---
 drivers/scsi/snic/snic_main.c | 1 +
 1 file changed, 1 insertion(+)

drivers/scsi/snic/snic_main.c +1 -0

index 82953e6a0915..cd638b4a4d7b 100644
@@ -305,6 +305,7 @@ snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
 	if (!shost->work_q) {
 		SNIC_HOST_ERR(shost, "Failed to Create ScsiHost wq.\n");
 
+		scsi_remove_host(shost);
 		ret = -ENOMEM;
 	}

Signature · 1 line

2.25.1

[email protected]

AI review

2026-07-27 07:48 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware resources are torn down before removing the SCSI host on the snic_probe() error path, leading to race condition and use-after-free.
- [High] Missing list_del(&snic->list) on the snic_probe() error path leaves a dangling pointer in the global snic_glob->snic_list, causing memory corruption.
- [High] Missing cancel_work_sync(&snic->link_work) on the snic_probe() error path allows asynchronously queued work to dereference a freed snic object. --

--- Patch [1]: [PATCH] scsi: snic: Fix scsi host leak on workqueue allocation failure --- commit 862e6c3dc0ab1bdd6e8457040994057a804388df
Author: Chen Changcheng <[email protected]>

scsi: snic: Fix scsi host leak on workqueue allocation failure

This commit fixes a memory leak where a failed workqueue allocation in snic_add_host() leaves the Scsi_Host registered in sysfs as a zombie device. It undoes the successful scsi_add_host() by calling scsi_remove_host() on the error path before returning.

drivers/scsi/snic/snic_main.c snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev) · line 310

Quoted 9 lines

drivers/scsi/snic/snic_main.c +1 -0

@@ -305,6 +305,7 @@ snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)
 	if (!shost->work_q) {
 		SNIC_HOST_ERR(shost, "Failed to Create ScsiHost wq.\n");
 
+		scsi_remove_host(shost);
 		ret = -ENOMEM;
 	}

[Severity: High]
This is a pre-existing issue, but does this error path in snic_probe() create a race condition?

If snic_disc_start() fails, the driver jumps to err_get_conf:

drivers/scsi/snic/snic_main.c:snic_probe() {
    ...
err_get_conf:
        snic_free_all_untagged_reqs(snic);
        for (i = 0; i < snic->intr_count; i++)
                svnic_intr_mask(&snic->intr[i]);
        snic_free_intr(snic);
err_req_intr:
        svnic_dev_disable(snic->vdev);
    ...
        snic_del_host(snic->shost);
}

The host is already fully registered via snic_add_host() and the state is SNIC_ONLINE. The hardware and interrupts are torn down, but snic_del_host() is delayed until the end. Could a concurrent sysfs scan or SCSI error handler queue commands to the disabled hardware during this window?

[Severity: High]
This isn't a bug introduced by this patch, but does the err_free_snic error path in snic_probe() leave a dangling pointer?

The snic structure is added to the global list during probe:

drivers/scsi/snic/snic_main.c:snic_probe() {
    ...
        spin_lock_irqsave(&snic_glob->snic_list_lock, flags);
        list_add_tail(&snic->list, &snic_glob->snic_list);
        spin_unlock_irqrestore(&snic_glob->snic_list_lock, flags);
    ...
}

If a subsequent step like snic_add_host() fails, the snic structure is freed via scsi_host_put():

drivers/scsi/snic/snic_main.c:snic_probe() {
    ...
err_free_snic:
        snic_stats_debugfs_remove(snic);
        scsi_host_put(shost);
}

Will the lack of list_del(&snic->list) before freeing cause memory corruption on subsequent device probes when list_add_tail() accesses the freed structure?

[Severity: High]
This is a pre-existing issue, but can the snic_probe() error path result in a use-after-free from asynchronously queued work?

Interrupts are requested and unmasked before configuring IO:

drivers/scsi/snic/snic_main.c:snic_probe() {
    ...
        for (i = 0; i < snic->intr_count; i++)
                svnic_intr_unmask(&snic->intr[i]);
    ...
}

A link event could trigger snic_handle_link_event(), which queues snic->link_work. If a subsequent probe step fails, the err_free_snic path masks interrupts and frees the snic structure:

drivers/scsi/snic/snic_main.c:snic_probe() {
    ...
err_free_snic:
        snic_stats_debugfs_remove(snic);
        scsi_host_put(shost);
}

Because the error path does not call cancel_work_sync(&snic->link_work) or flush the workqueue, could the pending work item execute later and dereference the freed snic object?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Narsimhulu Musini (nmusini)

maintainer Acked-by on v1 1/1

2026-07-27 23:51 UTC #2 lore raw

In reply to Chen Changcheng

________________________________________
From: Chen Changcheng <[email protected]>
Sent: 27 July 2026 12:34 AM
To: Karan Tilak Kumar (kartilak) <[email protected]>; Narsimhulu Musini (nmusini) <[email protected]>; Sesidhar Baddela (sebaddel) <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>
Cc: [email protected] <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Chen Changcheng <[email protected]>
Subject: [PATCH] scsi: snic: Fix scsi host leak on workqueue allocation failure

In snic_add_host(), if scsi_add_host() succeeds but
alloc_ordered_workqueue() fails, the function returns -ENOMEM
with shost->work_q left as NULL. The caller's error path then
calls snic_del_host(), which returns early when !shost->work_q
without calling scsi_remove_host(). The Scsi_Host remains
registered in sysfs as a zombie device even after the probe
has failed. This causes:

Before the fix:

  # /sys/class/scsi_host/ contains a zombie host0:
  $ cat /sys/class/scsi_host/host0/proc_name
  snic_scsi
  $ cat /sys/class/scsi_host/host0/state
  running
  # ata_piix gets host1, host2 (host0 stuck):
  scsi host1: ata_piix
  scsi host2: ata_piix

After the fix:

  # host0 is properly freed and reused by ata_piix:
  scsi host0: ata_piix
  scsi host1: ata_piix
  # No zombie host in /sys/class/scsi_host/

Signed-off-by Chen Changcheng <[email protected]>

Acked-by Narsimhulu Musini <[email protected]>

---
drivers/scsi/snic/snic_main.c | 1 +

 1 file changed, 1 insertion(+)

drivers/scsi/snic/snic_main.c +0 -0

index 82953e6a0915..cd638b4a4d7b 100644
@@ -305,6 +305,7 @@ snic_add_host(struct Scsi_Host *shost, struct pci_dev *pdev)

         if (!shost->work_q) {
                 SNIC_HOST_ERR(shost, "Failed to Create ScsiHost wq.\n");
+               scsi_remove_host(shost);
                 ret = -ENOMEM;
         }

--
2.25.1

Martin K. Petersen (Oracle)

maintainer Applied

2026-08-24 02:15 UTC #3 lore raw

In reply to Chen Changcheng

Chen,

Quoted 6 lines

In snic_add_host(), if scsi_add_host() succeeds but alloc_ordered_workqueue() fails, the function returns -ENOMEM with shost->work_q left as NULL. The caller's error path then calls snic_del_host(), which returns early when !shost->work_q without calling scsi_remove_host(). The Scsi_Host remains registered in sysfs as a zombie device even after the probe has failed. This causes:

Applied to 7.3/scsi-staging, thanks!

Signature · 1 line

Martin K. Petersen

Martin K. Petersen (Oracle)

maintainer Applied

2026-08-26 15:20 UTC #4 lore raw

In reply to Chen Changcheng

Quoted 9 lines · Chen Changcheng

In snic_add_host(), if scsi_add_host() succeeds but alloc_ordered_workqueue() fails, the function returns -ENOMEM with shost->work_q left as NULL. The caller's error path then calls snic_del_host(), which returns early when !shost->work_q without calling scsi_remove_host(). The Scsi_Host remains registered in sysfs as a zombie device even after the probe has failed. This causes:
[...]

Applied to 7.3/scsi-queue, thanks!

[1/1] scsi: snic: Fix scsi host leak on workqueue allocation failure

      https://git.kernel.org/mkp/scsi/c/12e67eb89eb2

Signature · 1 line

Martin K. Petersen