mbox series

[0/3] hw/net/smc91c111: Fix potential array overflows

Message ID 20250228174802.1945417-1-peter.maydell@linaro.org
Headers show
Series hw/net/smc91c111: Fix potential array overflows | expand

Message

Peter Maydell Feb. 28, 2025, 5:47 p.m. UTC 
This patchset fixes some potential array overflows in the
smc91c111 ethernet device model, including the one found in
https://gitlab.com/qemu-project/qemu/-/issues/2742

There are two classes of bugs:
 * we accept packet numbers from the guest, but we were not
   validating that they were in range before using them as an
   index into the data[][] array
 * we didn't sanitize the length field read from the data
   frame on tx before using it as an index to find the
   control byte at the end of the frame, so we could read off
   the end of the buffer

This patchset fixes both of these. The datasheet is sadly
silent on the h/w behaviour for these errors, so I opted to
LOG_GUEST_ERROR and silently ignore the invalid operations.

Patch 3 tidies up the existing code to use a constant defined
in patch 2; I put it last so we can cc the first two patches
to stable without having to also backport that patch.

thanks
-- PMM

Peter Maydell (3):
  hw/net/smc91c111: Sanitize packet numbers
  hw/net/smc91c111: Sanitize packet length on tx
  hw/net/smc91c111: Use MAX_PACKET_SIZE instead of magic numbers

 hw/net/smc91c111.c | 87 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 8 deletions(-)

Comments

Peter Maydell Feb. 28, 2025, 7:22 p.m. UTC | #1 
On Fri, 28 Feb 2025 at 17:48, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patchset fixes some potential array overflows in the
> smc91c111 ethernet device model, including the one found in
> https://gitlab.com/qemu-project/qemu/-/issues/2742
>
> There are two classes of bugs:
>  * we accept packet numbers from the guest, but we were not
>    validating that they were in range before using them as an
>    index into the data[][] array
>  * we didn't sanitize the length field read from the data
>    frame on tx before using it as an index to find the
>    control byte at the end of the frame, so we could read off
>    the end of the buffer
>
> This patchset fixes both of these. The datasheet is sadly
> silent on the h/w behaviour for these errors, so I opted to
> LOG_GUEST_ERROR and silently ignore the invalid operations.
>
> Patch 3 tidies up the existing code to use a constant defined
> in patch 2; I put it last so we can cc the first two patches
> to stable without having to also backport that patch.

See also the other smc91c111 fuzzer fix patch:
https://patchew.org/QEMU/20250228191652.1957208-1-peter.maydell@linaro.org/

(if I need to do a v2 of this series I'll put that one in too)

-- PMM