[RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Alex Bennée]

The datasheet doesn't explicitly say that TXFR_LEN has to be word
aligned but the fact there is a DMA_D_WIDTH flag to select between 32
bit and 128 bit strongly implies that is how it works. The downstream
rpi kernel also goes to efforts to not write sub-4 byte lengths so
lets:

  - fail when mis-programmed and report GUEST_ERROR
  - catch setting D_WIDTH for 128 bit and report UNIMP
  - add comments that the DEBUG register isn't a straight write

This includes the test case from the reported bug which is of unknown
provenience but isn't particularly novel.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3201
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 hw/dma/bcm2835_dma.c           | 19 +++++++++++++++++++
 tests/qtest/bcm2835-dma-test.c | 22 ++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/hw/dma/bcm2835_dma.c b/hw/dma/bcm2835_dma.c
index a2771ddcb52..e03247796cf 100644
--- a/hw/dma/bcm2835_dma.c
+++ b/hw/dma/bcm2835_dma.c
@@ -86,6 +86,19 @@ static void bcm2835_dma_update(BCM2835DMAState *s, unsigned c)
         }
         xlen_td = xlen;
 
+        if (ch->ti & BCM2708_DMA_D_WIDTH) {
+            qemu_log_mask(LOG_UNIMP, "%s: 128bit transfers not yet supported", __func__);
+            ch->cs |= BCM2708_DMA_ERR;
+            break;
+        }
+
+        /* datasheet implies 32bit or 128bit transfers only */
+        if (xlen & 0x3) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: bad transfer size\n", __func__);
+            ch->cs |= BCM2708_DMA_ERR;
+            break;
+        }
+
         while (ylen != 0) {
             /* Normal transfer mode */
             while (xlen != 0) {
@@ -229,6 +242,12 @@ static void bcm2835_dma_write(BCM2835DMAState *s, hwaddr offset,
         ch->conblk_ad = value;
         break;
     case BCM2708_DMA_DEBUG:
+        /* this needs masking
+         * 31:29 - WAZRI
+         * 28:04 - RO
+         *    03 - WAZRI
+         * 00:02 - W1CLR
+         */
         ch->debug = value;
         break;
     default:
diff --git a/tests/qtest/bcm2835-dma-test.c b/tests/qtest/bcm2835-dma-test.c
index 18901b76d21..4b100480f25 100644
--- a/tests/qtest/bcm2835-dma-test.c
+++ b/tests/qtest/bcm2835-dma-test.c
@@ -105,12 +105,34 @@ static void bcm2835_dma_test_interrupts(void)
     bcm2835_dma_test_interrupt(14, 11);
 }
 
+static void test_cve_underflow_txfr_len_1(void)
+{
+    uint64_t dma_base = RASPI3_DMA_BASE; // 0x3f007000
+    uint32_t cb_addr = 0x1000;
+    uint32_t src_addr = 0x2000;
+    uint32_t dst_addr = 0x3000;
+    /* Prepare DMA Control Block with VULNERABLE configuration */
+    writel(cb_addr + 0, BCM2708_DMA_S_INC | BCM2708_DMA_D_INC); /* TI */
+    writel(cb_addr + 4, src_addr); /* source address */
+    writel(cb_addr + 8, dst_addr); /* destination address */
+    writel(cb_addr + 12, 1); /* ⚠️ txfr_len = 1 (TRIGGER!) */
+    writel(cb_addr + 16, 0); /* stride */
+    writel(cb_addr + 20, 0); /* next CB = NULL */
+    /* Set control block address */
+    writel(dma_base + BCM2708_DMA_ADDR, cb_addr);
+    /* Trigger DMA - this will cause the vulnerability */
+    writel(dma_base + BCM2708_DMA_CS, BCM2708_DMA_ACTIVE);
+    /* Without the fix, QEMU process will hang at 100% CPU */
+}
+
 int main(int argc, char **argv)
 {
     int ret;
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/bcm2835/dma/test_interrupts",
                    bcm2835_dma_test_interrupts);
+    qtest_add_func("/bcm2835/dma/test_underflow_txfr",
+                   test_cve_underflow_txfr_len_1);
     qtest_start("-machine raspi3b");
     ret = g_test_run();
     qtest_end();
-- 
2.47.3

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Peter Maydell]

On Tue, 11 Nov 2025 at 10:54, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The datasheet doesn't explicitly say that TXFR_LEN has to be word
> aligned but the fact there is a DMA_D_WIDTH flag to select between 32
> bit and 128 bit strongly implies that is how it works. The downstream
> rpi kernel also goes to efforts to not write sub-4 byte lengths so
> lets:
>
>   - fail when mis-programmed and report GUEST_ERROR
>   - catch setting D_WIDTH for 128 bit and report UNIMP
>   - add comments that the DEBUG register isn't a straight write
>
> This includes the test case from the reported bug which is of unknown
> provenience but isn't particularly novel.

The emoji in the comment seems like a bit of a flag that
it might be AI-generated :-)

I would at least clean up the comments to be more in
line with our usual style and degree of detail.

thanks
-- PMM

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Yodel Eldar]

On 11/11/2025 04:54, Alex Bennée wrote:
> The datasheet doesn't explicitly say that TXFR_LEN has to be word
> aligned but the fact there is a DMA_D_WIDTH flag to select between 32
> bit and 128 bit strongly implies that is how it works. The downstream

At the bottom of page 38, the datasheet [1] states "the DMA can deal
with byte aligned transfers and will minimise bus traffic by buffering
and packing misaligned accesses."

IIUC, the *_WIDTH info fields are implied as maxima.

[1] https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

Regards,
Yodel

> rpi kernel also goes to efforts to not write sub-4 byte lengths so
> lets:

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Peter Maydell]

On Fri, 14 Nov 2025 at 16:06, Yodel Eldar <yodel.eldar@yodel.dev> wrote:
>
>
> On 11/11/2025 04:54, Alex Bennée wrote:
> > The datasheet doesn't explicitly say that TXFR_LEN has to be word
> > aligned but the fact there is a DMA_D_WIDTH flag to select between 32
> > bit and 128 bit strongly implies that is how it works. The downstream
>
> At the bottom of page 38, the datasheet [1] states "the DMA can deal
> with byte aligned transfers and will minimise bus traffic by buffering
> and packing misaligned accesses."

Yeah, I read that, but my interpretation of it is that it says
it's OK to provide non-4-aligned source and destination addresses.
It doesn't say anything either way about whether the transfer
size can be a non-multiple of 4.

thanks
-- PMM

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Alex Bennée]

Yodel Eldar <yodel.eldar@yodel.dev> writes:

(add Florians to CC)

> On 11/11/2025 04:54, Alex Bennée wrote:
>> The datasheet doesn't explicitly say that TXFR_LEN has to be word
>> aligned but the fact there is a DMA_D_WIDTH flag to select between 32
>> bit and 128 bit strongly implies that is how it works. The downstream
>
> At the bottom of page 38, the datasheet [1] states "the DMA can deal
> with byte aligned transfers and will minimise bus traffic by buffering
> and packing misaligned accesses."
>
> IIUC, the *_WIDTH info fields are implied as maxima.
>
> [1] https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

That reads ambiguously - you could start a misaligned n*WIDTH transfer
and the hardware will write bytes until aligned?

If it does indeed work with byte accesses maybe we can just do:

  if (xlen & 0x3) {
    .. do one byte ..
    xlen -= 1;
  } else {
    .. existing 32 bit code ..
  }

but I guess we need to handle unaligned accesses as well.

Florian,

Can you help clarify what the datasheet means here?

Thanks,

<snip>

>> rpi kernel also goes to efforts to not write sub-4 byte lengths so
>> lets:

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Florian Kauer]

On 11/14/25 17:43, Alex Bennée wrote:
> Yodel Eldar <yodel.eldar@yodel.dev> writes:
> 
> (add Florians to CC)
> 
>> On 11/11/2025 04:54, Alex Bennée wrote:
>>> The datasheet doesn't explicitly say that TXFR_LEN has to be word
>>> aligned but the fact there is a DMA_D_WIDTH flag to select between 32
>>> bit and 128 bit strongly implies that is how it works. The downstream
>>
>> At the bottom of page 38, the datasheet [1] states "the DMA can deal
>> with byte aligned transfers and will minimise bus traffic by buffering
>> and packing misaligned accesses."
>>
>> IIUC, the *_WIDTH info fields are implied as maxima.
>>
>> [1] https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf
> 
> That reads ambiguously - you could start a misaligned n*WIDTH transfer
> and the hardware will write bytes until aligned?
> 
> If it does indeed work with byte accesses maybe we can just do:
> 
>   if (xlen & 0x3) {
>     .. do one byte ..
>     xlen -= 1;
>   } else {
>     .. existing 32 bit code ..
>   }
> 
> but I guess we need to handle unaligned accesses as well.
> 
> Florian,
> 
> Can you help clarify what the datasheet means here?


Sorry, I won't be of big help here. I have no further insights apart from what the datasheet says.
I implemented this driver many years ago and it worked for my specific use case (enabling the I2S interface of the Raspberry Pi) this way,
but I cannot make a confident statement about unaligned access :-(

Greetings,
Florian


> 
> Thanks,
> 
> <snip>
> 
>>> rpi kernel also goes to efforts to not write sub-4 byte lengths so
>>> lets:
> 

Re: [RFC PATCH] hw/dma: don't allow weird transfer lengths for bcm2835

[Yodel Eldar]

On 14/11/2025 10:43, Alex Bennée wrote:
> Yodel Eldar <yodel.eldar@yodel.dev> writes:
> 
> (add Florians to CC)
> 
>> On 11/11/2025 04:54, Alex Bennée wrote:
>>> The datasheet doesn't explicitly say that TXFR_LEN has to be word
>>> aligned but the fact there is a DMA_D_WIDTH flag to select between 32
>>> bit and 128 bit strongly implies that is how it works. The downstream
>>
>> At the bottom of page 38, the datasheet [1] states "the DMA can deal
>> with byte aligned transfers and will minimise bus traffic by buffering
>> and packing misaligned accesses."
>>
>> IIUC, the *_WIDTH info fields are implied as maxima.
>>
>> [1] https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf
> 
> That reads ambiguously - you could start a misaligned n*WIDTH transfer
> and the hardware will write bytes until aligned?
> 
> If it does indeed work with byte accesses maybe we can just do:
> 
>    if (xlen & 0x3) {
>      .. do one byte ..
>      xlen -= 1;
>    } else {
>      .. existing 32 bit code ..
>    }
> 
> but I guess we need to handle unaligned accesses as well.
> 
> Florian,
> 
> Can you help clarify what the datasheet means here?
> 
> Thanks,
> 
> <snip>
> 
>>> rpi kernel also goes to efforts to not write sub-4 byte lengths so
>>> lets:
> 

Sorry for the lagged response: I was reviewing the datasheet and,
failing to find clarity there, familiarizing myself with the AXI
protocol spec and relevant driver code. Unfortunately, I'm still
uncertain about how the DMA controller handles unaligned values in
TXFR_LEN.XLENGTH, but below I've listed some of my abridged findings in
the hope that it may be of some help. Given my ambivalence regarding
the answer to the question, I defer to Alex and the community.

In alignment with Peter's comment, the AXI spec clearly explicates
support of unaligned start address transfers [1] but doesn't appear to
require support of unaligned ending transfers, although, that doesn't
preclude them either.

On the other hand, we know the BCM2835 supports write strobes at least
for triggering cache prefetching [2], and that this "allows memory
structures to be implemented that can be written using byte and half
word accesses" [3]. The specification also states "[a]ll [manager]
interfaces and interconnect must provide correct write strobes" and
that subordinate components can choose to: fully use them, ignore them
(i.e., "treat all writes as being the full data bus width"), or detect
unsupported write strobe combinations and provide an error response.
Moreover, "[a]ny [subordinate] component that is providing memory-like
behavior must fully support write strobes" [3]. To me, this suggests
that the BCM2835 has what it needs to be able to invalidate the bytes
past the TXFR_LEN bytes in the final beat of a DMA write transaction...
but IIUC it's possible to forego that and remain AXI-compliant.

Section 4.2 of {A}, Burst Length, also mentions the use of write strobes
for partial write transactions and extends it to cover read transactions
via discarding, too:

     No component can terminate a burst early to reduce the number of
     data transfers. During a write burst, the [manager] can disable
     further writing by deasserting all the write strobes, but it
     must complete the remaining transfers in the burst. During a
     read burst, the [manager] can discard further read data, but it
     must complete the remaining transfers in the burst. [4]

As a counterpoint, however, perhaps only whole transfers of a read
transaction can be discarded, or the BCM2835 forewent it altogether.

Narrow Transfers, section 9.3, also mentions the generation of transfers
that are narrower than the data bus of the manager [5].

All of the above are taken from the AXI spec, but as mentioned earlier
some of these features are opt-in, and the BCM2835 could have skipped
them. So, lastly, let's consider TXFR_LEN from the datasheet [6]: It's
described as "specif[ying] the amount of data to be transferred in
bytes." Moreover, all of the bits of its bitfield are accounted,
including TXFR_LEN[31:30]: "Reserved - Write as 0, read as don't care."
It could be an unfortunate oversight that the authors' didn't describe
TXFR_LEN[1:0] with the same description, or it could be intentional. If
XLENGTH's two LSBs are always 0 (or just ignored), then couldn't
TXFR_LEN simply indicate the number of transfers instead of the total
number of bytes transferred in a transaction, like YLENGTH in 2D mode?

I'm inclined to think that the line about the DMA dealing with byte
aligned transfers means that both unaligned start addresses and
a partial terminal data transfers via packing, buffering, discarding,
and the application of write strobes on full-width transfers are
possible, but without more precise language or a test on hardware (not
planned anytime soon), I can't be sure. Also, please let me know if
I misunderstood any of the source material or important details I
may have missed.

In the absence of certainty and for the sake of addressing the DoS issue
that inspired the patch, perhaps it's better to leave the patch as is
with a comment soliciting further investigation on actual behavior?

{A} AMBA AXI Protocol Version: 2.0 Specification
     https://documentation-service.arm.com/static/64256e84314e245d086bc88f

{B} BCM2835 ARM Peripherals
     https://datasheets.raspberrypi.com/bcm2835/bcm2835-peripherals.pdf

[1] {A} (p. 10-2)
[2] {B} (p. 51)
[3] {A} (p. 14-5)
[4] {A} (p. 4-3)
[5] {A} (p. 9-4)
[6] {B} (p. 53)

Thanks,
Yodel