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 <[email protected]> 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. > 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/[email protected]?part=1
