Re: [PATCH] scsi: check that the tag map is still there in scsi_host_find_tag()

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

Pre-existing issues:
- [High] The lockless checks for `shost->tag_set.tags` and `shost->tag_set.tags[hwq]` introduce a Time-of-Check to Time-of-Use (TOCTOU) race condition that fails to safely resolve the Use-After-Free and NULL pointer dereference vulnerabilities.
--

--- Patch [1]: [PATCH] scsi: check that the tag map is still there in scsi_host_find_tag() ---
commit 090ebf7d7f01fec40907d4c4480b8d9725dc44b2
Author: Yehyeong Lee <[email protected]>

scsi: check that the tag map is still there in scsi_host_find_tag()

This commit addresses a NULL pointer dereference in scsi_host_find_tag()
that occurs when a driver looks up a tag while the host is being removed.
It attempts to fix this by explicitly checking if shost->tag_set.tags and
shost->tag_set.tags[hwq] are non-NULL before dereferencing them.

> diff --git a/include/scsi/scsi_tcq.h b/include/scsi/scsi_tcq.h
> index ea7848e74d257..d62bae05d4e7d 100644
> --- a/include/scsi/scsi_tcq.h
> +++ b/include/scsi/scsi_tcq.h
> @@ -29,7 +29,8 @@ static inline struct scsi_cmnd *scsi_host_find_tag(struct Scsi_Host *shost,
>  		return NULL;
>  
>  	hwq = blk_mq_unique_tag_to_hwq(tag);
> -	if (hwq < shost->tag_set.nr_hw_queues) {
> +	if (hwq < shost->tag_set.nr_hw_queues && shost->tag_set.tags &&
> +	    shost->tag_set.tags[hwq]) {
>  		req = blk_mq_tag_to_rq(shost->tag_set.tags[hwq],
>  					blk_mq_unique_tag_to_tag(tag));
>  	}

[Severity: High]
This is a pre-existing issue, but does this lockless check leave a
time-of-check to time-of-use race condition unresolved?

The commit message notes that blk_mq_tagset_busy_iter() tests the array
before using it, but that iterator operates under an srcu_read_lock(). Here
in scsi_host_find_tag(), there is no synchronization.

Could a thread evaluate shost->tag_set.tags and shost->tag_set.tags[hwq] as
non-NULL, get preempted while blk_mq_free_tag_set() clears and frees the tags,
and then resume to dereference freed memory in blk_mq_tag_to_rq()?

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