Thread (3 messages) 3 messages, 3 authors, 14d ago

Re: [PATCH] spi: virtio: Use the per-transfer bits per word

From: sashiko-bot@kernel.org
Date: 2026-09-12 19:30:54

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

Pre-existing issues:
- [High] The driver unconditionally uses sg_init_one() on xfer->tx_buf and xfer->rx_buf, causing a crash or memory corruption if the SPI buffers are allocated via vmalloc.
--

commit c48c32e667f00c40f450ecae1a3af2defbfd7bc2
Author: Hao-Qun Huang [off-list ref]

spi: virtio: Use the per-transfer bits per word

This commit updates the virtio SPI driver to correctly use the per-transfer
bits_per_word value instead of the device default. This ensures that
transfers with specific word sizes are not sent with partial words that the
backend might reject.
quoted hunk ↗ jump to hunk
diff --git a/drivers/spi/spi-virtio.c b/drivers/spi/spi-virtio.c
index 2256dfec5407d..3e181bd8bc94b 100644
--- a/drivers/spi/spi-virtio.c
+++ b/drivers/spi/spi-virtio.c
@@ -168,7 +168,7 @@ static int virtio_spi_transfer_one(struct spi_controller *ctrl,
 
 	/* Fill struct spi_transfer_head */
 	th->chip_select_id = spi_get_chipselect(spi, 0);
-	th->bits_per_word = spi->bits_per_word;
+	th->bits_per_word = xfer->bits_per_word;
 	th->cs_change = xfer->cs_change;
 	th->tx_nbits = xfer->tx_nbits;
 	th->rx_nbits = xfer->rx_nbits;
[Severity: High]
This is a pre-existing issue, but looking further down in
virtio_spi_transfer_one(), the driver unconditionally passes the transfer
buffers to sg_init_one():

    if (spi_req->tx_buf) {
        sg_init_one(&sg_out_payload, spi_req->tx_buf, xfer->len);
        ...
    }

    if (spi_req->rx_buf) {
        sg_init_one(&sg_in_payload, spi_req->rx_buf, xfer->len);
        ...
    }

Does this cause memory corruption or a kernel crash if the SPI buffers are
allocated via vmalloc?

The SPI core automatically bounce-buffers vmalloc memory into scatterlists,
but only if the driver indicates DMA support by implementing the can_dma
callback.

Since spi-virtio does not implement can_dma, __spi_map_msg() skips mapping
and passes the virtual addresses directly to virtio_spi_transfer_one().

If a SPI client passes a vmalloc'd buffer, passing it to sg_init_one() will
use virt_to_page() to compute an invalid struct page pointer.

When virtqueue_add_sgs() maps this scatterlist for DMA, would it calculate
a bogus physical address based on this invalid page, causing the virtio
device to overwrite or read from arbitrary physical memory, or triggering
an IOMMU fault?

Would this mean any in-kernel client submitting a vmalloc'd SPI buffer could
trigger this issue, leading to an unsafe DMA mapping that compromises kernel
integrity?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913032049.11209.alvinhuang0603@gmail.com?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help