qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] fdc: check for illegal dma length calculation


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH] fdc: check for illegal dma length calculation
Date: Wed, 26 Jan 2022 17:39:21 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.0

Hi Jon,

On 26/1/22 16:44, Jon Maloy wrote:
On 1/13/22 20:33, Jon Maloy wrote:
The function fdctrl_start_transfer() calculates the dma data length
wrongly when certain boundary conditions are fulfilled. We have
noticed that the if ((fdctrl->fifo[5] - fdctrl->fifo[6]) > 1) we get
a dma length that will be interpreted as negative by the next function
in the chain, fdctrl_transfer_handler(). This leads to a crash.

If a security issue is reproducible (like the ones found by fuzzers),
please include the reproducer along.

Rather than trying to fix this obscure calculation, we just check if
the harmful condition is fulfilled, and return without action if that
is the case. Since this is a condition that can only be created by a
malicious user we deem this solution safe.

This fix is intended to address CVE-2021-3507.

Ah, this is similar to:
https://lore.kernel.org/qemu-devel/20211118115733.4038610-1-philmd@redhat.com/
which already contains the reproducer...
You might want to review it ;)

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
  hw/block/fdc.c | 5 +++++
  1 file changed, 5 insertions(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 21d18ac2e3..80a1f1750a 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1532,6 +1532,11 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
          if (fdctrl->fifo[0] & 0x80)
              tmp += fdctrl->fifo[6];
          fdctrl->data_len *= tmp;
+        if (tmp < 0) {
+            FLOPPY_DPRINTF("calculated illegal data_len=%u, tmp=%i\n",
+                           fdctrl->data_len, tmp);
+            return;
+        }
      }
      fdctrl->eot = fdctrl->fifo[6];
      if (fdctrl->dor & FD_DOR_DMAEN) {
I never received any feedback on this one.
Is there any?

Probably none so far because you forgot to Cc the maintainers:

$ ./scripts/get_maintainer.pl -f hw/block/fdc.c
John Snow <jsnow@redhat.com> (supporter:Floppy)
Kevin Wolf <kwolf@redhat.com> (supporter:Block layer core)
Hanna Reitz <hreitz@redhat.com> (supporter:Block layer core)
qemu-block@nongnu.org (open list:Floppy)
qemu-devel@nongnu.org (open list:All patches CC here)

(now Cc'ed).



reply via email to

[Prev in Thread] Current Thread [Next in Thread]