linux-kernel - Re: [PATCH] scsi: sd_zbc: Limit the report zones buffer size to UIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ff2ba1e1-a7bd-49b7-a1f2-e51f5cfed27a@kernel.org>
Date: Fri, 25 Apr 2025 10:42:46 +0900
From: Damien Le Moal <dlemoal@...nel.org>
To: "Siwinski, Steve" <ssiwinski@...o.com>,
 Christoph Hellwig <hch@...radead.org>
Cc: bgrove@...o.com, James.Bottomley@...senpartnership.com,
 linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
 martin.petersen@...cle.com, Steve Siwinski <stevensiwinski@...il.com>
Subject: Re: [PATCH] scsi: sd_zbc: Limit the report zones buffer size to
 UIO_MAXIOV

On 4/25/25 00:33, Siwinski, Steve wrote:
> My issue is not with passthough report zones.
> 
> The report zones command is failing on driver load and causing the drive 
> to fail to appear as a block device. If queue_max_segments is set to a 
> value over 1024, then nr_vecs in bio_alloc() will be greater than 
> UIO_MAXIOV and bio_alloc() will return NULL.

OK... A remainder about the path:

sd_zbc_do_report_zones() -> scsi_execute_cmd() -> blk_rq_map_kern() ->
bio_map_kern() -> bio_kmalloc()

and the fact that bio_kmalloc() does not allow more than UIO_MAXIOV segments
would have made things clear from the beginning. I had to look it up again to
understand why UIO_MAXIOV matters.

> This causes the error.
> ```
> sd 8:0:0:0: [sdb] REPORT ZONES start lba 0 failed
> sd 8:0:0:0: [sdb] REPORT ZONES: Result: hostbyte=0xff driverbyte=DRIVER_OK
> sdb: failed to revalidate zones
> ```
> 
> You can reproduce this by setting the max_sgl_entries parameter to 2k or 
> greater in the mpt3sas driver. Other drivers can also reproduce this 
> behavior.

Well, I think that the problem you uncovered here is a lot more fundamental than
just ZBC report zones. If the drive has a queue_max_segments() value larger than
UIO_MAXIOV, any attempt to map a large buffer for any command (e.g. a read) will
also fail. So this limit inconsistency seems wrong...

Christoph ? Since you were touching the vmalloc-ed BIO mapping code, do you have
any idea about this ? The quick and dirty fix would be to do:

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 7a447ff600d2..3cb897b25878 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -169,6 +169,7 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
                                        unsigned int nr_zones, size_t *buflen)
 {
        struct request_queue *q = sdkp->disk->queue;
+       size_t max_segs;
        size_t bufsize;
        void *buf;

@@ -185,7 +186,8 @@ static void *sd_zbc_alloc_report_buffer(struct scsi_disk *sdkp,
        bufsize = roundup((nr_zones + 1) * 64, SECTOR_SIZE);
        bufsize = min_t(size_t, bufsize,
                        queue_max_hw_sectors(q) << SECTOR_SHIFT);
-       bufsize = min_t(size_t, bufsize, queue_max_segments(q) << PAGE_SHIFT);
+       max_segs = min(queue_max_segments(q), UIO_MAXIOV);
+       bufsize = min_t(size_t, bufsize, max_segs << PAGE_SHIFT);

        while (bufsize >= SECTOR_SIZE) {
                buf = kvzalloc(bufsize, GFP_KERNEL | __GFP_NORETRY);


But that feels wrong...

-- 
Damien Le Moal
Western Digital Research