[PATCH-for-9.0?] hw/net/smc91c111: Fix out of bounds access in packets buffer

[PATCH-for-9.0?] hw/net/smc91c111: Fix out of bounds access in packets buffer

[Philippe Mathieu-Daudé]

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(-)

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.  */
-- 
2.41.0

Re: [PATCH-for-9.0?] hw/net/smc91c111: Fix out of bounds access in packets buffer

[Philippe Mathieu-Daudé]

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.  */