Re: [PATCH v2 2/3] scsi: sd: fix special_vec mempool leak when scsi_alloc_sgtables() fails

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The special payload page can be freed into the wrong mempool if the device sector size changes while the command is in flight.
--

--- Patch [2]: [PATCH v2 2/3] scsi: sd: fix special_vec mempool leak when scsi_alloc_sgtables() fails ---
commit 39421d4b78837e533a8958996d1edabedfa78d22
Author: Yang Xiuwei <yangxiuwei@kylinos.cn>

scsi: sd: fix special_vec mempool leak when scsi_alloc_sgtables() fails

This commit fixes a mempool leak when scsi_alloc_sgtables() fails in
sd_setup_unmap_cmnd() or sd_setup_write_same{10,16}_cmnd(). It ensures
sd_uninit_command() is called on error, clearing RQF_SPECIAL_PAYLOAD
and freeing the payload page.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index d18693d390b2f..8fed1cda9ac8c 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -958,6 +958,21 @@ static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
>  	return protect;
>  }
>  
> +static void sd_uninit_command(struct scsi_cmnd *cmd)
> +{
> +	struct request *rq = scsi_cmd_to_rq(cmd);
> +	struct scsi_device *sdp = cmd->device;
> +
> +	if (!(rq->rq_flags & RQF_SPECIAL_PAYLOAD))
> +		return;
> +
> +	if (sdp->sector_size > PAGE_SIZE)
> +		mempool_free(rq->special_vec.bv_page, sd_large_page_pool);
> +	else
> +		mempool_free(rq->special_vec.bv_page, sd_page_pool);

[Severity: High]
This isn't a bug introduced by this patch, but can this free the page to the
wrong mempool if the device sector size changes while the command is in flight?

sd_set_special_bvec() allocates the page based on the sector size at the time
of submission:

    if (sector_size > PAGE_SIZE)
        page = mempool_alloc(sd_large_page_pool, GFP_ATOMIC);
    else
        page = mempool_alloc(sd_page_pool, GFP_ATOMIC);

If sd_read_capacity() updates sdp->sector_size dynamically during a concurrent
rescan before the request queue is frozen, this cleanup path might read the
updated sector size. This could result in freeing an order-0 page into the
large page pool or vice versa, causing memory corruption upon subsequent
allocations from the pool.

> +	rq->rq_flags &= ~RQF_SPECIAL_PAYLOAD;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707030333.22245-1-yangxiuwei@kylinos.cn?part=2