On 2020/7/10 下午7:07, Peter Maydell wrote:
On Fri, 10 Jul 2020 at 10:20, Mauro Matteo Cascella <mcasc...@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 <ezrak...@gmail.com> Signed-off-by: Mauro Matteo Cascella <mcasc...@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, sizeof(frame)); 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 ? thanks -- 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? Thanks [1] https://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_xgmac
