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

[PATCH v3] 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.

Add zeroing pad_len bytes of padding by pad_offset offset before
increasing its length. This ensures that the DMA does not receive
uninitialized data and eliminates the KMSAN warning.

In this case, the pages are not located in highmem, but in the
general case they might be, so kmap_local_page() is used for mapping.

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.
---
 drivers/scsi/scsi_lib.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 22e2e3223..c75b86059 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1187,6 +1187,11 @@ 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 pad_offset = last_sg->offset + last_sg->length;
+		void *vaddr = kmap_local_page(sg_page(last_sg));
+
+		memset(vaddr + pad_offset, 0, pad_len);
+		kunmap_local(vaddr);
 
 		last_sg->length += pad_len;
 		cmd->extra_len += pad_len;
-- 
2.49.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Out-of-bounds virtual memory access on highmem systems due to single-page `kmap_local_page` mapping combined with multi-page scatterlist elements.
--

commit 3b4fc492120b7d6d0be664dd4aa56d3b359efb8f
Author: Petr Vaganov <p.vaganov@ideco.ru>

scsi: fill in DMA padding bytes in scsi_alloc_sgtables

This commit zero-pads the remaining unaligned bytes at the end of the last
scatterlist element to prevent DMAing uninitialized memory. It maps the page
using kmap_local_page() to support highmem configurations.

> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1187,6 +1187,11 @@ 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 pad_offset = last_sg->offset + last_sg->length;
> +		void *vaddr = kmap_local_page(sg_page(last_sg));
> +
> +		memset(vaddr + pad_offset, 0, pad_len);

[Severity: Critical]
Can this regression cause an out-of-bounds write if the scatterlist element
spans multiple coalesced pages?

If an I/O request has a data buffer spanning multiple physically contiguous
pages, the block layer coalesces them into a single scatterlist entry. When
this happens, last_sg->length will exceed PAGE_SIZE.

If the request length is not aligned to dma_pad_mask, pad_offset (which is
calculated as last_sg->offset + last_sg->length) will be greater than
PAGE_SIZE. Since kmap_local_page() only maps the very first page of the
scatterlist element, the memset() would write past the mapped page and corrupt
adjacent memory slots.

> +		kunmap_local(vaddr);
>  
>  		last_sg->length += pad_len;
>  		cmd->extra_len += pad_len;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628171348.8613-1-p.vaganov@ideco.ru?part=1