[Top][All Lists]

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

Re: [PATCH] hw/net/xgmac: Fix buffer overflow in xgmac_enet_send()

From: Jason Wang
Subject: Re: [PATCH] hw/net/xgmac: Fix buffer overflow in xgmac_enet_send()
Date: Tue, 14 Jul 2020 17:09:23 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

On 2020/7/10 下午7:07, Peter Maydell wrote:
On Fri, 10 Jul 2020 at 10:20, Mauro Matteo Cascella <mcascell@redhat.com> wrote:
A buffer overflow issue was reported by Mr. Ziming Zhang, CC'd here. It
occurs while sending an Ethernet frame due to missing break statements
and improper checking of the buffer size.

Reported-by: Ziming Zhang <ezrakiez@gmail.com>
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
  hw/net/xgmac.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c
index 574dd47b41..b872afbb1a 100644
--- a/hw/net/xgmac.c
+++ b/hw/net/xgmac.c
@@ -224,17 +224,20 @@ static void xgmac_enet_send(XgmacState *s)
              DEBUGF_BRK("qemu:%s:ERROR...ERROR...ERROR... -- "
                          "xgmac buffer 1 len on send > 2048 (0x%x)\n",
                           __func__, bd.buffer1_size & 0xfff);
+            break;
          if ((bd.buffer2_size & 0xfff) != 0) {
              DEBUGF_BRK("qemu:%s:ERROR...ERROR...ERROR... -- "
                          "xgmac buffer 2 len on send != 0 (0x%x)\n",
                          __func__, bd.buffer2_size & 0xfff);
+            break;
-        if (len >= sizeof(frame)) {
+        if (frame_size + len >= sizeof(frame)) {
              DEBUGF_BRK("qemu:%s: buffer overflow %d read into %zu "
-                        "buffer\n" , __func__, len, sizeof(frame));
+                        "buffer\n" , __func__, frame_size + len, 
              DEBUGF_BRK("qemu:%s: buffer1.size=%d; buffer2.size=%d\n",
                          __func__, bd.buffer1_size, bd.buffer2_size);
+            break;

          cpu_physical_memory_read(bd.buffer1_addr, ptr, len);
This is correct in the sense that it avoids the buffer overflow.

I suspect that we should probably also be reporting the error
back to the guest via some kind of error flag in the descriptor
and/or in a status register. Unfortunately I don't have a copy
of the datasheet and it doesn't seem to be available online :-(
Does anybody have a copy to check ?

-- PMM

I tried to download the datasheet from [1] but it's not a programmer manual.

I think we can apply this patch first and do follow-up fixes on top?


[1] https://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_xgmac

reply via email to

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