[PATCH 2/3] ftgmac100: Improve DMA error handling

Cédric Le Goater posted 3 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH 2/3] ftgmac100: Improve DMA error handling
Posted by Cédric Le Goater 1 week, 5 days ago
Currently, DMA memory operation errors in the ftgmac100 model are not
all tested and this can lead to a guest-triggerable denial of service
as described in https://gitlab.com/qemu-project/qemu/-/work_items/3335.

To fix this, check the return value of ftgmac100_write_bd() in the TX
path and exit the TX loop on error to prevent further processing. In
the event of a DMA error, also set FTGMAC100_INT_AHB_ERR interrupt
flag as appropriate.

The FTGMAC100_INT_AHB_ERR interrupt status bit only applies to the
AST2400 SoC; on newer Aspeed SoCs, it is a reserved bit.
Nevertheless, since it is supported by the Linux driver and it should
be safe to use in the QEMU implementation across all SoCs.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3335
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/net/ftgmac100.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 69a38adb627f..25c3ede72cbb 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -624,7 +624,10 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint64_t tx_ring,
         bd.des0 &= ~FTGMAC100_TXDES0_TXDMA_OWN;
 
         /* Write back the modified descriptor.  */
-        ftgmac100_write_bd(&bd, addr);
+        if (ftgmac100_write_bd(&bd, addr)) {
+            s->isr |= FTGMAC100_INT_AHB_ERR;
+            break;
+        }
         /* Advance to the next descriptor.  */
         if (bd.des0 & s->txdes0_edotr) {
             addr = tx_ring;
@@ -1134,7 +1137,10 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const uint8_t *buf,
             bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
             s->isr |= FTGMAC100_INT_RPKT_BUF;
         }
-        ftgmac100_write_bd(&bd, addr);
+        if (ftgmac100_write_bd(&bd, addr)) {
+            s->isr |= FTGMAC100_INT_AHB_ERR;
+            break;
+        }
         if (bd.des0 & s->rxdes0_edorr) {
             addr = s->rx_ring;
         } else {
-- 
2.53.0


Re: [PATCH 2/3] ftgmac100: Improve DMA error handling
Posted by Philippe Mathieu-Daudé 1 week, 4 days ago
On 22/3/26 22:57, Cédric Le Goater wrote:
> Currently, DMA memory operation errors in the ftgmac100 model are not
> all tested and this can lead to a guest-triggerable denial of service
> as described in https://gitlab.com/qemu-project/qemu/-/work_items/3335.
> 
> To fix this, check the return value of ftgmac100_write_bd() in the TX
> path and exit the TX loop on error to prevent further processing. In
> the event of a DMA error, also set FTGMAC100_INT_AHB_ERR interrupt
> flag as appropriate.
> 
> The FTGMAC100_INT_AHB_ERR interrupt status bit only applies to the
> AST2400 SoC; on newer Aspeed SoCs, it is a reserved bit.
> Nevertheless, since it is supported by the Linux driver and it should
> be safe to use in the QEMU implementation across all SoCs.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3335
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/net/ftgmac100.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

RE: [PATCH 2/3] ftgmac100: Improve DMA error handling
Posted by Jamin Lin 1 week, 4 days ago
> -----Original Message-----
> From: Cédric Le Goater <clg@redhat.com>
> Sent: Monday, March 23, 2026 5:58 AM
> To: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org; Peter Maydell <peter.maydell@linaro.org>; Jamin
> Lin <jamin_lin@aspeedtech.com>; Kane Chen <kane_chen@aspeedtech.com>;
> Cédric Le Goater <clg@redhat.com>
> Subject: [PATCH 2/3] ftgmac100: Improve DMA error handling
> 
> Currently, DMA memory operation errors in the ftgmac100 model are not all
> tested and this can lead to a guest-triggerable denial of service as described in
> https://gitlab.com/qemu-project/qemu/-/work_items/3335.
> 
> To fix this, check the return value of ftgmac100_write_bd() in the TX path and
> exit the TX loop on error to prevent further processing. In the event of a DMA
> error, also set FTGMAC100_INT_AHB_ERR interrupt flag as appropriate.
> 
> The FTGMAC100_INT_AHB_ERR interrupt status bit only applies to the
> AST2400 SoC; on newer Aspeed SoCs, it is a reserved bit.
> Nevertheless, since it is supported by the Linux driver and it should be safe to
> use in the QEMU implementation across all SoCs.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3335
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  hw/net/ftgmac100.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index
> 69a38adb627f..25c3ede72cbb 100644
> --- a/hw/net/ftgmac100.c
> +++ b/hw/net/ftgmac100.c
> @@ -624,7 +624,10 @@ static void ftgmac100_do_tx(FTGMAC100State *s,
> uint64_t tx_ring,
>          bd.des0 &= ~FTGMAC100_TXDES0_TXDMA_OWN;
> 
>          /* Write back the modified descriptor.  */
> -        ftgmac100_write_bd(&bd, addr);
> +        if (ftgmac100_write_bd(&bd, addr)) {
> +            s->isr |= FTGMAC100_INT_AHB_ERR;
> +            break;
> +        }
>          /* Advance to the next descriptor.  */
>          if (bd.des0 & s->txdes0_edotr) {
>              addr = tx_ring;
> @@ -1134,7 +1137,10 @@ static ssize_t ftgmac100_receive(NetClientState
> *nc, const uint8_t *buf,
>              bd.des0 |= flags | FTGMAC100_RXDES0_LRS;
>              s->isr |= FTGMAC100_INT_RPKT_BUF;
>          }
> -        ftgmac100_write_bd(&bd, addr);
> +        if (ftgmac100_write_bd(&bd, addr)) {
> +            s->isr |= FTGMAC100_INT_AHB_ERR;
> +            break;
> +        }
>          if (bd.des0 & s->rxdes0_edorr) {
>              addr = s->rx_ring;
>          } else {
> --
> 2.53.0

Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>

Thanks,
Jamin