[PATCH] scsi: myrb: unregister the SCSI host on remove

[PATCH] scsi: myrb: unregister the SCSI host on remove

[Wentao Liang]

scsi_add_host() takes a reference on the host device that is only
dropped by scsi_remove_host() when the host and its children are
unregistered.  myrb_remove() goes straight to myrb_cleanup(), which only
drops the reference taken by scsi_host_alloc(), so the host stays
registered after the PCI device is removed and the memory behind it is
leaked.

Call scsi_remove_host() before tearing the controller down so that no
new commands are queued and the host is unregistered.

Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
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/myrb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index 95df6e5345f7..e43d86f139c1 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -3496,6 +3496,7 @@ static void myrb_remove(struct pci_dev *pdev)
 {
 	struct myrb_hba *cb = pci_get_drvdata(pdev);
 
+	scsi_remove_host(cb->host);
 	shost_printk(KERN_NOTICE, cb->host, "Flushing Cache...");
 	myrb_exec_type3(cb, MYRB_CMD_FLUSH, 0);
 	myrb_cleanup(cb);
-- 
2.34.1

Re: [PATCH] scsi: myrb: unregister the SCSI host on remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] scsi: myrb: UAF of cb and active monitor_work in myrb_remove() teardown
--

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

scsi: myrb: unregister the SCSI host on remove

This commit adds a call to scsi_remove_host() in myrb_remove() so that the
SCSI host is unregistered and its device registration references are dropped
when the PCI device is removed.

> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index 95df6e5345f7a..e43d86f139c12 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c
> @@ -3496,6 +3496,7 @@ static void myrb_remove(struct pci_dev *pdev)
>  {
>  	struct myrb_hba *cb = pci_get_drvdata(pdev);
>  
> +	scsi_remove_host(cb->host);
>  	shost_printk(KERN_NOTICE, cb->host, "Flushing Cache...");
>  	myrb_exec_type3(cb, MYRB_CMD_FLUSH, 0);
>  	myrb_cleanup(cb);

[Severity: Critical]
Does calling scsi_remove_host(cb->host) here trigger a use-after-free of cb
when myrb_remove() subsequently calls myrb_destroy_mempools(cb)?

In drivers/scsi/myrb.c:myrb_detect(), struct myrb_hba *cb is allocated inline
within struct Scsi_Host via scsi_host_alloc() and obtained with
shost_priv(shost):

drivers/scsi/myrb.c:myrb_detect() {
	shost = scsi_host_alloc(&myrb_template, sizeof(struct myrb_hba));
	if (!shost) {
		dev_err(&pdev->dev, "Unable to allocate Controller\n");
		return NULL;
	}
	shost->max_cmd_len = 12;
	shost->max_lun = 256;
	cb = shost_priv(shost);
}

Now that scsi_remove_host(cb->host) drops the device registration references
taken by scsi_add_host(), the scsi_host_put(cb->host) call at the end of
myrb_cleanup(cb) drops the final reference on shost->shost_gendev and frees
both shost and cb via scsi_host_dev_release():

drivers/scsi/myrb.c:myrb_cleanup() {
	...
	pci_set_drvdata(pdev, NULL);
	pci_disable_device(pdev);
	scsi_host_put(cb->host);
}

Immediately after myrb_cleanup(cb) returns, myrb_remove() calls
myrb_destroy_mempools(cb), which dereferences the freed cb pointer:

drivers/scsi/myrb.c:myrb_remove() {
	struct myrb_hba *cb = pci_get_drvdata(pdev);

	scsi_remove_host(cb->host);
	shost_printk(KERN_NOTICE, cb->host, "Flushing Cache...");
	myrb_exec_type3(cb, MYRB_CMD_FLUSH, 0);
	myrb_cleanup(cb);
	myrb_destroy_mempools(cb);
}

drivers/scsi/myrb.c:myrb_destroy_mempools() {
	cancel_delayed_work_sync(&cb->monitor_work);
	destroy_workqueue(cb->work_q);

	dma_pool_destroy(cb->sg_pool);
	dma_pool_destroy(cb->dcdb_pool);
}

In addition, because cb->monitor_work and cb->work_q are only stopped in
myrb_destroy_mempools(), running myrb_cleanup() first leaves myrb_monitor()
able to run concurrently while myrb_cleanup() -> myrb_unmap() frees the DMA
buffers and mailboxes (cb->enquiry, cb->err_table, cb->ldev_info_buf,
cb->first_stat_mbox, cb->first_cmd_mbox), unmaps cb->mmio_base, and frees the
IRQ.

Should myrb_destroy_mempools(cb) be called before myrb_cleanup(cb), matching
the order used in the myrb_probe() error path and in myrs_remove()?

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

Re: [PATCH] scsi: myrb: unregister the SCSI host on remove

[Hannes Reinecke]

On 11/4/26 3:14 AM, Wentao Liang wrote:
> scsi_add_host() takes a reference on the host device that is only
> dropped by scsi_remove_host() when the host and its children are
> unregistered.  myrb_remove() goes straight to myrb_cleanup(), which only
> drops the reference taken by scsi_host_alloc(), so the host stays
> registered after the PCI device is removed and the memory behind it is
> leaked.
> 
> Call scsi_remove_host() before tearing the controller down so that no
> new commands are queued and the host is unregistered.
> 
> Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
> 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/myrb.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
> index 95df6e5345f7..e43d86f139c1 100644
> --- a/drivers/scsi/myrb.c
> +++ b/drivers/scsi/myrb.c
> @@ -3496,6 +3496,7 @@ static void myrb_remove(struct pci_dev *pdev)
>   {
>   	struct myrb_hba *cb = pci_get_drvdata(pdev);
>   
> +	scsi_remove_host(cb->host);
>   	shost_printk(KERN_NOTICE, cb->host, "Flushing Cache...");
>   	myrb_exec_type3(cb, MYRB_CMD_FLUSH, 0);
>   	myrb_cleanup(cb);

Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH] scsi: myrb: unregister the SCSI host on remove

[Krzysztof Kozlowski]

On 04/11/2026 08:54, Hannes Reinecke wrote:
> On 11/4/26 3:14 AM, Wentao Liang wrote:
>> scsi_add_host() takes a reference on the host device that is only
>> dropped by scsi_remove_host() when the host and its children are
>> unregistered.  myrb_remove() goes straight to myrb_cleanup(), which only
>> drops the reference taken by scsi_host_alloc(), so the host stays
>> registered after the PCI device is removed and the memory behind it is
>> leaked.
>>
>> Call scsi_remove_host() before tearing the controller down so that no
>> new commands are queued and the host is unregistered.
>>
>> Fixes: 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")
>> 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/myrb.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
>> index 95df6e5345f7..e43d86f139c1 100644
>> --- a/drivers/scsi/myrb.c
>> +++ b/drivers/scsi/myrb.c
>> @@ -3496,6 +3496,7 @@ static void myrb_remove(struct pci_dev *pdev)
>>   {
>>   	struct myrb_hba *cb = pci_get_drvdata(pdev);
>>   
>> +	scsi_remove_host(cb->host);
>>   	shost_printk(KERN_NOTICE, cb->host, "Flushing Cache...");
>>   	myrb_exec_type3(cb, MYRB_CMD_FLUSH, 0);
>>   	myrb_cleanup(cb);
> 
> Reviewed-by: Hannes Reinecke <hare@kernel.org>

You sure? Sashiko reports introduced UAF. 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: myrb: unregister the SCSI host on remove

[Wentao Liang]

Hi Krzysztof, Hannes, 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