Re: [PATCH] ata: libata-eh: queue hotplug work on the system_dfl_long_wq workqueue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch fails its stated goal of unifying workqueues, leaving a Use-After-Free concurrency vulnerability intact due to broken workqueue non-reentrancy.
--

commit 30d96f652048a28eb19aca7f2ef201c2aea08917
Author: Niklas Cassel <cassel@kernel.org>

ata: libata-eh: queue hotplug work on the system_dfl_long_wq workqueue

This commit changes ata_scsi_port_error_handler() to queue hotplug_task
on system_dfl_long_wq, attempting to unify the workqueue usage with
ata_scsi_scan_host().

>     ata_scsi_scan_host() queues the ap->hotplug_task work on the unbound
>     system_dfl_long_wq workqueue.

Is this description of ata_scsi_scan_host() accurate?

Looking at ata_scsi_scan_host(), it queues the work on system_long_wq, not
the unbound system_dfl_long_wq.

> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 9a4b67b90b17b..6cb79a09423dd 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -819,7 +819,7 @@ void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap)
>  		ap->pflags &= ~ATA_PFLAG_LOADING;
>  	else if ((ap->pflags & ATA_PFLAG_SCSI_HOTPLUG) &&
>  		!(ap->flags & ATA_FLAG_SAS_HOST))
> -		schedule_delayed_work(&ap->hotplug_task, 0);
> +		queue_delayed_work(system_dfl_long_wq, &ap->hotplug_task, 0);

Could this still leave the port vulnerable to a use-after-free regression
during teardown?

Since ata_scsi_scan_host() uses system_long_wq and this now uses
system_dfl_long_wq, the same work can still be queued on two different
workqueues, bypassing the workqueue non-reentrancy checks.

If the work was last queued to system_dfl_long_wq, would a call to
cancel_delayed_work_sync(&ap->hotplug_task) during ata_port_detach() only
flush that pool and return immediately, missing an instance still executing
on system_long_wq?

Should this be changed to use system_long_wq to match ata_scsi_scan_host()
and ensure the synchronization actually waits for all work to complete?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513081001.714630-2-cassel@kernel.org?part=1