All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Petr Vaganov" <p.vaganov@ideco.ru>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v3] scsi: fill in DMA padding bytes in scsi_alloc_sgtables
Date: Sun, 28 Jun 2026 17:32:19 +0000	[thread overview]
Message-ID: <20260628173220.416271F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260628171348.8613-1-p.vaganov@ideco.ru>

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

      reply	other threads:[~2026-06-28 17:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-28 17:13 [PATCH v3] scsi: fill in DMA padding bytes in scsi_alloc_sgtables Petr Vaganov
2026-06-28 17:32 ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260628173220.416271F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=p.vaganov@ideco.ru \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.