[PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[Wentao Liang]

pm8001_init_ccb_tag() is called after the Scsi_Host, the sas_ha and the
driver's private data have been allocated, and after the PCI regions
have been requested.  When it fails, pm8001_pci_probe() jumps to
err_out_enable, which only returns the error: the Scsi_Host, sas_ha and
private data are leaked, the PCI regions stay requested and the device
stays enabled.

Jump to err_out_ha_free instead, which releases all of those resources.

Fixes: 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding I/O supported to 1024")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/scsi/pm8001/pm8001_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 5af81c73a8f8..389649b36aab 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1178,7 +1178,7 @@ static int pm8001_pci_probe(struct pci_dev *pdev,
 
 	rc = pm8001_init_ccb_tag(pm8001_ha);
 	if (rc)
-		goto err_out_enable;
+		goto err_out_ha_free;
 
 
 	PM8001_CHIP_DISP->chip_post_init(pm8001_ha);
-- 
2.34.1

Re: [PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] scsi: pm8001: use-after-free via active IRQs in pm8001_pci_probe()
- [High] scsi: pm8001: DMA regions freed while MPI is active in pm8001_pci_probe()
- [Medium] scsi: pm8001: memory leaks on ccb tag failure unwind in pm8001_pci_probe()
--

commit 06b680982a72c843fb06984b05c4494852d0cf98
Author: Wentao Liang <vulab@iscas.ac.cn>

scsi: pm8001: unwind the probe on ccb tag allocation failure

This commit changes the error handling path in pm8001_pci_probe() when
pm8001_init_ccb_tag() fails, jumping to err_out_ha_free instead of
err_out_enable to release previously allocated resources.

> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 5af81c73a8f87..389649b36aabd 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1178,7 +1178,7 @@ static int pm8001_pci_probe(struct pci_dev *pdev,
>  
>  	rc = pm8001_init_ccb_tag(pm8001_ha);
>  	if (rc)
> -		goto err_out_enable;
> +		goto err_out_ha_free;

[Severity: High]
Can jumping to err_out_ha_free here leave active interrupt handlers
registered with dangling pointers into freed pm8001_ha and sha memory?

Before pm8001_init_ccb_tag() runs, pm8001_pci_probe() calls
pm8001_pci_alloc() -> pm8001_init_tasklet() and pm8001_alloc() ->
pm8001_request_irq().

In MSI-X mode, pm8001_request_msix() registers IRQ handlers pointing into
pm8001_ha:

drivers/scsi/pm8001/pm8001_init.c:pm8001_request_msix() {
	...
		rc = request_irq(pci_irq_vector(pm8001_ha->pdev, i),
			pm8001_interrupt_handler_msix, flag,
			pm8001_ha->intr_drvname[i],
			&(pm8001_ha->irq_vector[i]));
	...
}

In INT-X mode, pm8001_request_irq() registers a shared IRQ handler with
pm8001_ha->name and sha:

drivers/scsi/pm8001/pm8001_init.c:pm8001_request_irq() {
	...
	return request_irq(pdev->irq, pm8001_interrupt_handler_intx,
			   IRQF_SHARED, pm8001_ha->name,
			   SHOST_TO_SAS_HA(pm8001_ha->shost));
}

When pm8001_init_ccb_tag() fails, the err_out_ha_free path calls
pm8001_free(pm8001_ha) and kfree(sha) without calling
pm8001_free_irq(pm8001_ha) or pm8001_kill_tasklet(pm8001_ha) (unlike
pm8001_pci_remove()):

drivers/scsi/pm8001/pm8001_init.c:pm8001_pci_probe() {
	...
err_out_ha_free:
	pm8001_free(pm8001_ha);
err_out_free:
	kfree(sha);
	...
}

If a shared INT-X interrupt fires afterwards, pm8001_interrupt_handler_intx()
will dereference the freed sha and pm8001_ha structures and read unmapped MMIO
registers, and reading /proc/interrupts will dereference the freed devname
strings in pm8001_ha->intr_drvname[i] or pm8001_ha->name.

[Severity: High]
Also on the err_out_ha_free path, does pm8001_free() release coherent DMA
memory while the controller MPI engine is still active?

In pm8001_pci_probe(), PM8001_CHIP_DISP->chip_init(pm8001_ha) succeeds right
before pm8001_init_ccb_tag() is called, which programs the physical DMA
addresses of pm8001_ha->memoryMap.region[] (inbound/outbound queues,
producer/consumer indices, and firmware event logs) into the controller and
enables the firmware MPI state machine via mpi_init_check().

When pm8001_init_ccb_tag() fails and jumps to err_out_ha_free, pm8001_free()
immediately frees those DMA regions without first calling
PM8001_CHIP_DISP->chip_soft_rst(pm8001_ha) to uninitialize the MPI engine and
reset the chip (as pm8001_pci_remove() does before calling pm8001_free()):

drivers/scsi/pm8001/pm8001_init.c:pm8001_free() {
	...
	for (i = 0; i < USI_MAX_MEMCNT; i++) {
		if (pm8001_ha->memoryMap.region[i].virt_ptr != NULL) {
			dma_free_coherent(&pm8001_ha->pdev->dev,
				(pm8001_ha->memoryMap.region[i].total_len +
				pm8001_ha->memoryMap.region[i].alignment),
				pm8001_ha->memoryMap.region[i].virt_ptr,
				pm8001_ha->memoryMap.region[i].phys_addr);
			}
	}
	PM8001_CHIP_DISP->chip_iounmap(pm8001_ha);
	...
}

Should chip_soft_rst() be called before pm8001_free() so the controller cannot
DMA into the freed memoryMap.region[] buffers?

[Severity: Medium]
Finally, does unwinding to err_out_ha_free on pm8001_init_ccb_tag() failure
still leak sha->sas_phy, sha->sas_port, pm8001_ha->devices,
pm8001_ha->ccb_info, and partially allocated pm8001_ha->ccb_info[i].buf_prd
DMA buffers?

Inside pm8001_init_ccb_tag(), if bitmap_zalloc() for pm8001_ha->rsvd_tags
fails, or if dma_alloc_coherent() for pm8001_ha->ccb_info[i].buf_prd fails at
iteration i, it jumps to err_out:

drivers/scsi/pm8001/pm8001_init.c:pm8001_init_ccb_tag() {
	...
	pm8001_ha->rsvd_tags = bitmap_zalloc(PM8001_RESERVE_SLOT, GFP_KERNEL);
	if (!pm8001_ha->rsvd_tags)
		goto err_out;
	...
	for (i = 0; i < ccb_count; i++) {
		pm8001_ha->ccb_info[i].buf_prd = dma_alloc_coherent(dev,
				sizeof(struct pm8001_prd) * PM8001_MAX_DMA_SG,
				&pm8001_ha->ccb_info[i].ccb_dma_handle,
				GFP_KERNEL);
		if (!pm8001_ha->ccb_info[i].buf_prd) {
			pm8001_dbg(pm8001_ha, FAIL,
				   "ccb prd memory allocation error\n");
			goto err_out;
		}
		...
	}

	return 0;

err_out_noccb:
	kfree(pm8001_ha->devices);
err_out:
	return -ENOMEM;
}

Jumping to err_out bypasses kfree(pm8001_ha->devices) (allocated in
pm8001_alloc()) and leaves pm8001_ha->ccb_info and any already allocated
pm8001_ha->ccb_info[0..i-1].buf_prd buffers unfreed. Neither pm8001_free() nor
err_out_free frees those allocations, nor do they free sha->sas_phy and
sha->sas_port (allocated in pm8001_prep_sas_ha_init()) before kfree(sha) is
called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021411.1467970-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[Jinpu Wang]

On Wed, Nov 4, 2026 at 3:14 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> pm8001_init_ccb_tag() is called after the Scsi_Host, the sas_ha and the
> driver's private data have been allocated, and after the PCI regions
> have been requested.  When it fails, pm8001_pci_probe() jumps to
> err_out_enable, which only returns the error: the Scsi_Host, sas_ha and
> private data are leaked, the PCI regions stay requested and the device
> stays enabled.
>
> Jump to err_out_ha_free instead, which releases all of those resources.
>
> Fixes: 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding I/O supported to 1024")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>
Thx!
> ---
>  drivers/scsi/pm8001/pm8001_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 5af81c73a8f8..389649b36aab 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1178,7 +1178,7 @@ static int pm8001_pci_probe(struct pci_dev *pdev,
>
>         rc = pm8001_init_ccb_tag(pm8001_ha);
>         if (rc)
> -               goto err_out_enable;
> +               goto err_out_ha_free;
>
>
>         PM8001_CHIP_DISP->chip_post_init(pm8001_ha);
> --
> 2.34.1
>

Re: [PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[Krzysztof Kozlowski]

On Wed, Nov 04, 2026 at 09:33:55AM +0100, Jinpu Wang wrote:
> On Wed, Nov 4, 2026 at 3:14 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
> >
> > pm8001_init_ccb_tag() is called after the Scsi_Host, the sas_ha and the
> > driver's private data have been allocated, and after the PCI regions
> > have been requested.  When it fails, pm8001_pci_probe() jumps to
> > err_out_enable, which only returns the error: the Scsi_Host, sas_ha and
> > private data are leaked, the PCI regions stay requested and the device
> > stays enabled.
> >
> > Jump to err_out_ha_free instead, which releases all of those resources.
> >
> > Fixes: 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding I/O supported to 1024")
> > Cc: stable@vger.kernel.org
> > Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>

Are you sure? Sashiko reports introduced UAF (and/or other bugs).

Mind that vulab@iscas.ac.cn is a bot with spams with tons of patches
which looks exactly like previous hypocrite commit approach - some
security vulnerability intentionally added in between of thousands of
valid patches.

Best regards,
Krzysztof

Re: [PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[Jinpu Wang]

On Wed, Nov 4, 2026 at 9:41 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Nov 04, 2026 at 09:33:55AM +0100, Jinpu Wang wrote:
> > On Wed, Nov 4, 2026 at 3:14 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
> > >
> > > pm8001_init_ccb_tag() is called after the Scsi_Host, the sas_ha and the
> > > driver's private data have been allocated, and after the PCI regions
> > > have been requested.  When it fails, pm8001_pci_probe() jumps to
> > > err_out_enable, which only returns the error: the Scsi_Host, sas_ha and
> > > private data are leaked, the PCI regions stay requested and the device
> > > stays enabled.
> > >
> > > Jump to err_out_ha_free instead, which releases all of those resources.
> > >
> > > Fixes: 5a141315ed7c ("scsi: pm80xx: Increase the number of outstanding I/O supported to 1024")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> > > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> > Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>
>
> Are you sure? Sashiko reports introduced UAF (and/or other bugs).
>
> Mind that vulab@iscas.ac.cn is a bot with spams with tons of patches
> which looks exactly like previous hypocrite commit approach - some
> security vulnerability intentionally added in between of thousands of
> valid patches.
oh, didn't notice the report, thanks for the reminder.
I take back my ack.
>
> Best regards,
> Krzysztof
>

Re: [PATCH] scsi: pm8001: unwind the probe on ccb tag allocation failure

[Wentao Liang]

Hi Krzysztof, Jack, all,

I am very sorry for the consequences of these submissions. You are right
to question the safety of this patch and the way we have handled these
submissions. I apologize for the inadequate validation, the volume of
patches, and our failure to respond to earlier feedback. That has wasted
reviewers' time and damaged trust in our contributions. I take
responsibility for my submissions.

I want to state clearly that there was no malicious intent behind these
patches. Our intention was to fix bugs, and we did not intend to insert
security vulnerabilities or deceive reviewers. I understand why the
combination of potentially unsafe changes, repeated submissions, and
unanswered feedback raised this concern. Intent alone does not make an
unsafe patch acceptable.

We did use AI assistance, but the responsibility for understanding and
checking the changes remains with us. We should not have treated a
single local reference-counting change as sufficient evidence that a
patch was safe.

Once again, I am sorry for my actions, and I will do everything I can to
address the harm caused by these submissions. I promise to respect the
requested posting pause and will not send further batches during this
period. I will personally reply to all of the feedback I have left
unanswered over the past month or two. Until I have completed those
replies and received your permission to resume, I will not submit any
new patches. I hope you will give me the opportunity to make things
right.

Best regards,
Wentao Liang