| Message ID | 20240408102711.58485-1-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-for-9.0?] hw/net/smc91c111: Fix out of bounds access in packets buffer | expand |
On 8/4/24 12:27, Philippe Mathieu-Daudé wrote: > While the Packet Number Register is 6-bit wide and could hold > up to 64 packets [*] our implementation is clamped at 4 packets. > > Reproducer: > > $ cat << EOF | qemu-system-arm -display none \ > -machine mainstone,accel=qtest \ > -qtest stdio > outl 0xcf8 0x80000010 > outl 0xcfc 0x10000300 > outl 0xcf8 0x80000004 > outl 0xcfc 0x07 > writel 0x1000030c 0x66027cd6 > writel 0x10000300 0x64af8eda > readw 0x10000308 > EOF > hw/net/smc91c111.c:607:24: runtime error: > index 175 out of bounds for type 'uint8_t[4][2048]' (aka 'unsigned char[4][2048]') > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ================================================================= > ==397944==ERROR: AddressSanitizer: SEGV on unknown address 0x629000077db4 > (pc 0x56272aed3b8d bp 0x7ffd1471f290 sp 0x7ffd1471ea20 T0) > ==397944==The signal is caused by a READ memory access. > #0 0x56272aed3b8d in smc91c111_readb hw/net/smc91c111.c:607:24 > #1 0x56272aecfd61 in smc91c111_readfn hw/net/smc91c111.c:650:16 > #2 0x56272d4b228b in memory_region_read_accessor system/memory.c:445:11 > #3 0x56272d46fb85 in access_with_adjusted_size system/memory.c:573:18 > #4 0x56272d46c58e in memory_region_dispatch_read1 system/memory.c:1426:16 > #5 0x56272d46bcd7 in memory_region_dispatch_read system/memory.c:1459:9 > #6 0x56272d4e8e03 in flatview_read_continue_step system/physmem.c:2794:18 > #7 0x56272d4e871e in flatview_read_continue system/physmem.c:2835:19 > #8 0x56272d4e98b8 in flatview_read system/physmem.c:2865:12 > #9 0x56272d4e9388 in address_space_read_full system/physmem.c:2878:18 > #10 0x56272d6e7840 in address_space_read include/exec/memory.h:3026:18 > ... > > Broken since model introduction in commit 80337b66a8. > > [*] LAN91C111 DS00002276A.pdf, chapter 8.17, Packet Number Register > > Reported-by: Will Lester Apparently also Reported-by: Chuhong Yuan <hslester96@gmail.com> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2268 > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/net/smc91c111.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c > index 702d0e8e83..286298bf06 100644 > --- a/hw/net/smc91c111.c > +++ b/hw/net/smc91c111.c > @@ -429,7 +429,7 @@ static void smc91c111_writeb(void *opaque, hwaddr offset, > /* Ignore. */ > return; > case 2: /* Packet Number Register */ > - s->packet_num = value; > + s->packet_num = value & (NUM_PACKETS - 1); > return; > case 3: case 4: case 5: > /* Should be readonly, but linux writes to them anyway. Ignore. */
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c index 702d0e8e83..286298bf06 100644 --- a/hw/net/smc91c111.c +++ b/hw/net/smc91c111.c @@ -429,7 +429,7 @@ static void smc91c111_writeb(void *opaque, hwaddr offset, /* Ignore. */ return; case 2: /* Packet Number Register */ - s->packet_num = value; + s->packet_num = value & (NUM_PACKETS - 1); return; case 3: case 4: case 5: /* Should be readonly, but linux writes to them anyway. Ignore. */
While the Packet Number Register is 6-bit wide and could hold up to 64 packets [*] our implementation is clamped at 4 packets. Reproducer: $ cat << EOF | qemu-system-arm -display none \ -machine mainstone,accel=qtest \ -qtest stdio outl 0xcf8 0x80000010 outl 0xcfc 0x10000300 outl 0xcf8 0x80000004 outl 0xcfc 0x07 writel 0x1000030c 0x66027cd6 writel 0x10000300 0x64af8eda readw 0x10000308 EOF hw/net/smc91c111.c:607:24: runtime error: index 175 out of bounds for type 'uint8_t[4][2048]' (aka 'unsigned char[4][2048]') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ================================================================= ==397944==ERROR: AddressSanitizer: SEGV on unknown address 0x629000077db4 (pc 0x56272aed3b8d bp 0x7ffd1471f290 sp 0x7ffd1471ea20 T0) ==397944==The signal is caused by a READ memory access. #0 0x56272aed3b8d in smc91c111_readb hw/net/smc91c111.c:607:24 #1 0x56272aecfd61 in smc91c111_readfn hw/net/smc91c111.c:650:16 #2 0x56272d4b228b in memory_region_read_accessor system/memory.c:445:11 #3 0x56272d46fb85 in access_with_adjusted_size system/memory.c:573:18 #4 0x56272d46c58e in memory_region_dispatch_read1 system/memory.c:1426:16 #5 0x56272d46bcd7 in memory_region_dispatch_read system/memory.c:1459:9 #6 0x56272d4e8e03 in flatview_read_continue_step system/physmem.c:2794:18 #7 0x56272d4e871e in flatview_read_continue system/physmem.c:2835:19 #8 0x56272d4e98b8 in flatview_read system/physmem.c:2865:12 #9 0x56272d4e9388 in address_space_read_full system/physmem.c:2878:18 #10 0x56272d6e7840 in address_space_read include/exec/memory.h:3026:18 ... Broken since model introduction in commit 80337b66a8. [*] LAN91C111 DS00002276A.pdf, chapter 8.17, Packet Number Register Reported-by: Will Lester Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2268 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/net/smc91c111.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)