* [PATCH] rtl8139: fix large_send_mss divide-by-zero
@ 2023-04-13 17:19 Stefan Hajnoczi
2023-04-13 18:24 ` Peter Maydell
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-04-13 17:19 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, Stefan Hajnoczi, Alexander Bulekov, Peter Maydell
If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
Even if the division wasn't a problem, the for loop that emits MSS-sized
packets would never terminate.
Solve these issues by skipping offloading when large_send_mss=0.
This issue was found by OSS-Fuzz as part of Alexander Bulekov's device
fuzzing work. The reproducer is:
$ cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
512M,slots=1,maxmem=0xffff000000000000 -machine q35 -nodefaults -device \
rtl8139,netdev=net0 -netdev user,id=net0 -device \
pc-dimm,id=nv1,memdev=mem1,addr=0xb800a64602800000 -object \
memory-backend-ram,id=mem1,size=2M -qtest stdio
outl 0xcf8 0x80000814
outl 0xcfc 0xe0000000
outl 0xcf8 0x80000804
outw 0xcfc 0x06
write 0xe0000037 0x1 0x04
write 0xe00000e0 0x2 0x01
write 0x1 0x1 0x04
write 0x3 0x1 0x98
write 0xa 0x1 0x8c
write 0xb 0x1 0x02
write 0xc 0x1 0x46
write 0xd 0x1 0xa6
write 0xf 0x1 0xb8
write 0xb800a646028c000c 0x1 0x08
write 0xb800a646028c000e 0x1 0x47
write 0xb800a646028c0010 0x1 0x02
write 0xb800a646028c0017 0x1 0x06
write 0xb800a646028c0036 0x1 0x80
write 0xe00000d9 0x1 0x40
EOF
Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1582
Fixes: 6d71357a3b65 ("rtl8139: honor large send MSS value")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/rtl8139.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 5a5aaf868d..5f1a4d359b 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -2154,6 +2154,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
int large_send_mss = (txdw0 >> CP_TC_LGSEN_MSS_SHIFT) &
CP_TC_LGSEN_MSS_MASK;
+ if (large_send_mss == 0) {
+ goto skip_offload;
+ }
DPRINTF("+++ C+ mode offloaded task TSO IP data %d "
"frame data %d specified MSS=%d\n",
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] rtl8139: fix large_send_mss divide-by-zero
2023-04-13 17:19 [PATCH] rtl8139: fix large_send_mss divide-by-zero Stefan Hajnoczi
@ 2023-04-13 18:24 ` Peter Maydell
2023-04-14 3:13 ` Jason Wang
2023-04-14 9:17 ` Michael Tokarev
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2023-04-13 18:24 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Jason Wang, Alexander Bulekov
On Thu, 13 Apr 2023 at 18:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
> Even if the division wasn't a problem, the for loop that emits MSS-sized
> packets would never terminate.
>
> Solve these issues by skipping offloading when large_send_mss=0.
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 5a5aaf868d..5f1a4d359b 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2154,6 +2154,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
>
> int large_send_mss = (txdw0 >> CP_TC_LGSEN_MSS_SHIFT) &
> CP_TC_LGSEN_MSS_MASK;
> + if (large_send_mss == 0) {
> + goto skip_offload;
> + }
Looks like 0 is the only problematic value for the code, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rtl8139: fix large_send_mss divide-by-zero
2023-04-13 18:24 ` Peter Maydell
@ 2023-04-14 3:13 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2023-04-14 3:13 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, qemu-devel, Alexander Bulekov
On Fri, Apr 14, 2023 at 2:24 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 13 Apr 2023 at 18:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
> > Even if the division wasn't a problem, the for loop that emits MSS-sized
> > packets would never terminate.
> >
> > Solve these issues by skipping offloading when large_send_mss=0.
>
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index 5a5aaf868d..5f1a4d359b 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -2154,6 +2154,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
> >
> > int large_send_mss = (txdw0 >> CP_TC_LGSEN_MSS_SHIFT) &
> > CP_TC_LGSEN_MSS_MASK;
> > + if (large_send_mss == 0) {
> > + goto skip_offload;
> > + }
>
> Looks like 0 is the only problematic value for the code, so
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I think it's not worth 8.0. So I've queued this.
If anyone think it is, Peter may queue this directly with
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtl8139: fix large_send_mss divide-by-zero
2023-04-13 17:19 [PATCH] rtl8139: fix large_send_mss divide-by-zero Stefan Hajnoczi
2023-04-13 18:24 ` Peter Maydell
@ 2023-04-14 9:17 ` Michael Tokarev
2023-04-14 10:10 ` Alexander Bulekov
2023-04-14 13:54 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: Michael Tokarev @ 2023-04-14 9:17 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Jason Wang, Alexander Bulekov, Peter Maydell, qemu-stable
13.04.2023 20:19, Stefan Hajnoczi wrote:
> If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
> Even if the division wasn't a problem, the for loop that emits MSS-sized
> packets would never terminate.
>
> Solve these issues by skipping offloading when large_send_mss=0.
Cc: qemu-stable@nongnu.org.
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1582
> Fixes: 6d71357a3b65 ("rtl8139: honor large send MSS value")
/mjt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rtl8139: fix large_send_mss divide-by-zero
2023-04-13 17:19 [PATCH] rtl8139: fix large_send_mss divide-by-zero Stefan Hajnoczi
2023-04-13 18:24 ` Peter Maydell
2023-04-14 9:17 ` Michael Tokarev
@ 2023-04-14 10:10 ` Alexander Bulekov
2023-04-14 13:54 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2023-04-14 10:10 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Jason Wang, Peter Maydell
On 230413 1319, Stefan Hajnoczi wrote:
> If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
> Even if the division wasn't a problem, the for loop that emits MSS-sized
> packets would never terminate.
>
> Solve these issues by skipping offloading when large_send_mss=0.
>
> This issue was found by OSS-Fuzz as part of Alexander Bulekov's device
> fuzzing work. The reproducer is:
>
> $ cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> 512M,slots=1,maxmem=0xffff000000000000 -machine q35 -nodefaults -device \
> rtl8139,netdev=net0 -netdev user,id=net0 -device \
> pc-dimm,id=nv1,memdev=mem1,addr=0xb800a64602800000 -object \
> memory-backend-ram,id=mem1,size=2M -qtest stdio
> outl 0xcf8 0x80000814
> outl 0xcfc 0xe0000000
> outl 0xcf8 0x80000804
> outw 0xcfc 0x06
> write 0xe0000037 0x1 0x04
> write 0xe00000e0 0x2 0x01
> write 0x1 0x1 0x04
> write 0x3 0x1 0x98
> write 0xa 0x1 0x8c
> write 0xb 0x1 0x02
> write 0xc 0x1 0x46
> write 0xd 0x1 0xa6
> write 0xf 0x1 0xb8
> write 0xb800a646028c000c 0x1 0x08
> write 0xb800a646028c000e 0x1 0x47
> write 0xb800a646028c0010 0x1 0x02
> write 0xb800a646028c0017 0x1 0x06
> write 0xb800a646028c0036 0x1 0x80
> write 0xe00000d9 0x1 0x40
> EOF
>
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1582
Maybe instead:
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1582
so that gitlab will auto-close the issue.
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Thank you
-Alex
> Fixes: 6d71357a3b65 ("rtl8139: honor large send MSS value")
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/net/rtl8139.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 5a5aaf868d..5f1a4d359b 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -2154,6 +2154,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
>
> int large_send_mss = (txdw0 >> CP_TC_LGSEN_MSS_SHIFT) &
> CP_TC_LGSEN_MSS_MASK;
> + if (large_send_mss == 0) {
> + goto skip_offload;
> + }
>
> DPRINTF("+++ C+ mode offloaded task TSO IP data %d "
> "frame data %d specified MSS=%d\n",
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] rtl8139: fix large_send_mss divide-by-zero
2023-04-13 17:19 [PATCH] rtl8139: fix large_send_mss divide-by-zero Stefan Hajnoczi
` (2 preceding siblings ...)
2023-04-14 10:10 ` Alexander Bulekov
@ 2023-04-14 13:54 ` Philippe Mathieu-Daudé
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-04-14 13:54 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Jason Wang, Alexander Bulekov, Peter Maydell
On 13/4/23 19:19, Stefan Hajnoczi wrote:
> If the driver sets large_send_mss to 0 then a divide-by-zero occurs.
> Even if the division wasn't a problem, the for loop that emits MSS-sized
> packets would never terminate.
>
> Solve these issues by skipping offloading when large_send_mss=0.
>
> This issue was found by OSS-Fuzz as part of Alexander Bulekov's device
> fuzzing work. The reproducer is:
>
> $ cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
> 512M,slots=1,maxmem=0xffff000000000000 -machine q35 -nodefaults -device \
> rtl8139,netdev=net0 -netdev user,id=net0 -device \
> pc-dimm,id=nv1,memdev=mem1,addr=0xb800a64602800000 -object \
> memory-backend-ram,id=mem1,size=2M -qtest stdio
> outl 0xcf8 0x80000814
> outl 0xcfc 0xe0000000
> outl 0xcf8 0x80000804
> outw 0xcfc 0x06
> write 0xe0000037 0x1 0x04
> write 0xe00000e0 0x2 0x01
> write 0x1 0x1 0x04
> write 0x3 0x1 0x98
> write 0xa 0x1 0x8c
> write 0xb 0x1 0x02
> write 0xc 0x1 0x46
> write 0xd 0x1 0xa6
> write 0xf 0x1 0xb8
> write 0xb800a646028c000c 0x1 0x08
> write 0xb800a646028c000e 0x1 0x47
> write 0xb800a646028c0010 0x1 0x02
> write 0xb800a646028c0017 0x1 0x06
> write 0xb800a646028c0036 0x1 0x80
> write 0xe00000d9 0x1 0x40
> EOF
>
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1582
> Fixes: 6d71357a3b65 ("rtl8139: honor large send MSS value")
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/net/rtl8139.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-14 13:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-13 17:19 [PATCH] rtl8139: fix large_send_mss divide-by-zero Stefan Hajnoczi
2023-04-13 18:24 ` Peter Maydell
2023-04-14 3:13 ` Jason Wang
2023-04-14 9:17 ` Michael Tokarev
2023-04-14 10:10 ` Alexander Bulekov
2023-04-14 13:54 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).