[PATCH v5] scsi: fill in DMA padding bytes in scsi_alloc_sgtables

[PATCH v5] scsi: fill in DMA padding bytes in scsi_alloc_sgtables

[Petr Vaganov]

During fuzz testing, the following issue was discovered:

BUG: KMSAN: uninit-value in __dma_map_sg_attrs+0x217/0x310
 __dma_map_sg_attrs+0x217/0x310
 dma_map_sg_attrs+0x4a/0x70
 ata_qc_issue+0x9f8/0x1420
 __ata_scsi_queuecmd+0x1657/0x1740
 ata_scsi_queuecmd+0x79a/0x920
 scsi_queue_rq+0x4472/0x4f40
 blk_mq_dispatch_rq_list+0x1cca/0x3ee0
 __blk_mq_sched_dispatch_requests+0x458/0x630
 blk_mq_sched_dispatch_requests+0x15b/0x340
 __blk_mq_run_hw_queue+0xe5/0x250
 __blk_mq_delay_run_hw_queue+0x138/0x780
 blk_mq_run_hw_queue+0x4bb/0x7e0
 blk_mq_sched_insert_request+0x2a7/0x4c0
 blk_execute_rq+0x497/0x8a0
 sg_io+0xbe0/0xe20
 scsi_ioctl+0x2b36/0x3c60
 sr_block_ioctl+0x319/0x440
 blkdev_ioctl+0x80f/0xd70
 __se_sys_ioctl+0x219/0x420
 __x64_sys_ioctl+0x93/0xe0
 x64_sys_call+0x1d6c/0x3ad0
 do_syscall_64+0x4c/0xa0
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Uninit was created at:
 __alloc_pages+0x5c0/0xc80
 alloc_pages+0xe0e/0x1050
 blk_rq_map_user_iov+0x2b77/0x6100
 blk_rq_map_user_io+0x2fa/0x4d0
 sg_io+0xad6/0xe20
 scsi_ioctl+0x2b36/0x3c60
 sr_block_ioctl+0x319/0x440
 blkdev_ioctl+0x80f/0xd70
 __se_sys_ioctl+0x219/0x420
 __x64_sys_ioctl+0x93/0xe0
 x64_sys_call+0x1d6c/0x3ad0
 do_syscall_64+0x4c/0xa0
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Bytes 14-15 of 16 are uninitialized
Memory access of size 16 starts at ffff88800cbdb000

When processing the last unaligned element of the scatterlist,
it is supplemented with missing bytes in the amount of pad_len.
These bytes remain uninitialized, which leads to a problem.

Extend last_sg->length by pad_len first, then use sg_zero_buffer() to
zero those pad_len bytes.  sg_zero_buffer() uses sg_miter internally,
which correctly handles sg entries spanning multiple pages and padding
that crosses a page boundary.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 40b01b9bbdf5 ("block: update bio according to DMA alignment padding")
Cc: stable@vger.kernel.org
Signed-off-by: Petr Vaganov <p.vaganov@ideco.ru>
---
v2: Added tag "Cc: stable@vger.kernel.org".
v3: Resending this patch as the issue is still present in the current
    kernel and the previous submission did not receive review.
v4: Use pfn_to_page()/page_to_pfn() arithmetic to locate the correct
    page when the last sg element spans multiple pages, fixing a
    potential out-of-bounds write.
    Use memzero_page() instead of open-coded kmap/memset/kunmap.
    Handle the case where padding crosses a page boundary.
v5: Replace hand-rolled page mapping with sg_zero_buffer(), which
    handles multi-page sg entries and page boundary splits correctly
    via sg_miter.
---
 drivers/scsi/scsi_lib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 22e2e3223..686cef240 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1187,8 +1187,10 @@ blk_status_t scsi_alloc_sgtables(struct scsi_cmnd *cmd)
 	if (blk_rq_bytes(rq) & rq->q->limits.dma_pad_mask) {
 		unsigned int pad_len =
 			(rq->q->limits.dma_pad_mask & ~blk_rq_bytes(rq)) + 1;
+		unsigned int data_len = last_sg->length;
 
 		last_sg->length += pad_len;
+		sg_zero_buffer(last_sg, 1, pad_len, data_len);
 		cmd->extra_len += pad_len;
 	}
 
-- 
2.49.0

Re: [PATCH v5] scsi: fill in DMA padding bytes in scsi_alloc_sgtables

[Bart Van Assche]

On 6/28/26 11:52 AM, Petr Vaganov wrote:
> During fuzz testing, the following issue was discovered:
> 
> BUG: KMSAN: uninit-value in __dma_map_sg_attrs+0x217/0x310
>   __dma_map_sg_attrs+0x217/0x310
>   dma_map_sg_attrs+0x4a/0x70
>   ata_qc_issue+0x9f8/0x1420
>   __ata_scsi_queuecmd+0x1657/0x1740
>   ata_scsi_queuecmd+0x79a/0x920
>   scsi_queue_rq+0x4472/0x4f40
>   blk_mq_dispatch_rq_list+0x1cca/0x3ee0
>   __blk_mq_sched_dispatch_requests+0x458/0x630
>   blk_mq_sched_dispatch_requests+0x15b/0x340
>   __blk_mq_run_hw_queue+0xe5/0x250
>   __blk_mq_delay_run_hw_queue+0x138/0x780
>   blk_mq_run_hw_queue+0x4bb/0x7e0
>   blk_mq_sched_insert_request+0x2a7/0x4c0
>   blk_execute_rq+0x497/0x8a0
>   sg_io+0xbe0/0xe20
>   scsi_ioctl+0x2b36/0x3c60
>   sr_block_ioctl+0x319/0x440
>   blkdev_ioctl+0x80f/0xd70
>   __se_sys_ioctl+0x219/0x420
>   __x64_sys_ioctl+0x93/0xe0
>   x64_sys_call+0x1d6c/0x3ad0
>   do_syscall_64+0x4c/0xa0
>   entry_SYSCALL_64_after_hwframe+0x6e/0xd8

dma_map_sg_attrs() shouldn't touch the data buffer. Is the above warning
triggered because of the kmsan_handle_dma_sg() call in
__dma_map_sg_attrs()?


>   		last_sg->length += pad_len;
> +		sg_zero_buffer(last_sg, 1, pad_len, data_len);
>   		cmd->extra_len += pad_len;

Shouldn't sg_zero_buffer() only be called if CONFIG_UBSAN is enabled to
prevent that this call negatively affects I/O performance?

Thanks,

Bart.

Re: [PATCH v5] scsi: fill in DMA padding bytes in scsi_alloc_sgtables

[Petr Vaganov]

Yes, the warning is triggered by kmsan_handle_dma_sg(). However, the 
KMSAN report does not appear to be a false positive. There is an actual 
initialization issue: scsi_alloc_sgtables() extends the last scatterlist 
entry by pad_len, but these additional bytes are not initialized before 
the buffer is mapped for DMA. As a result, uninitialized kernel memory 
may become visible to the device during a DMA_TO_DEVICE transfer.

This is exactly the kind of issue KMSAN is intended to detect. The KMSAN 
documentation explicitly mentions passing uninitialized memory to 
hardware as a security issue.

Initializing the padding unconditionally does add some overhead. 
However, sg_zero_buffer() is only called for requests that actually 
require DMA padding. In my opinion, this overhead is preferable to 
leaving uninitialized kernel memory exposed to the device.

Thanks,
Petr

On 7/23/26 01:27, Bart Van Assche wrote:
> On 6/28/26 11:52 AM, Petr Vaganov wrote:
>> During fuzz testing, the following issue was discovered:
>>
>> BUG: KMSAN: uninit-value in __dma_map_sg_attrs+0x217/0x310
>>   __dma_map_sg_attrs+0x217/0x310
>>   dma_map_sg_attrs+0x4a/0x70
>>   ata_qc_issue+0x9f8/0x1420
>>   __ata_scsi_queuecmd+0x1657/0x1740
>>   ata_scsi_queuecmd+0x79a/0x920
>>   scsi_queue_rq+0x4472/0x4f40
>>   blk_mq_dispatch_rq_list+0x1cca/0x3ee0
>>   __blk_mq_sched_dispatch_requests+0x458/0x630
>>   blk_mq_sched_dispatch_requests+0x15b/0x340
>>   __blk_mq_run_hw_queue+0xe5/0x250
>>   __blk_mq_delay_run_hw_queue+0x138/0x780
>>   blk_mq_run_hw_queue+0x4bb/0x7e0
>>   blk_mq_sched_insert_request+0x2a7/0x4c0
>>   blk_execute_rq+0x497/0x8a0
>>   sg_io+0xbe0/0xe20
>>   scsi_ioctl+0x2b36/0x3c60
>>   sr_block_ioctl+0x319/0x440
>>   blkdev_ioctl+0x80f/0xd70
>>   __se_sys_ioctl+0x219/0x420
>>   __x64_sys_ioctl+0x93/0xe0
>>   x64_sys_call+0x1d6c/0x3ad0
>>   do_syscall_64+0x4c/0xa0
>>   entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> dma_map_sg_attrs() shouldn't touch the data buffer. Is the above warning
> triggered because of the kmsan_handle_dma_sg() call in
> __dma_map_sg_attrs()?
>
>
>>           last_sg->length += pad_len;
>> +        sg_zero_buffer(last_sg, 1, pad_len, data_len);
>>           cmd->extra_len += pad_len;
>
> Shouldn't sg_zero_buffer() only be called if CONFIG_UBSAN is enabled to
> prevent that this call negatively affects I/O performance?
>
> Thanks,
>
> Bart. 

Re: [PATCH v5] scsi: fill in DMA padding bytes in scsi_alloc_sgtables

[Bart Van Assche]

On 6/28/26 11:52 AM, Petr Vaganov wrote:
> Extend last_sg->length by pad_len first, then use sg_zero_buffer() to
> zero those pad_len bytes.  sg_zero_buffer() uses sg_miter internally,
> which correctly handles sg entries spanning multiple pages and padding
> that crosses a page boundary.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>