| Message ID | 20250228174802.1945417-1-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/net/smc91c111: Fix potential array overflows | expand |
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
Ping for review of this series (and the other smc91c111 patch I link below), please? thanks -- PMM On Fri, 28 Feb 2025 at 19:22, Peter Maydell <peter.maydell@linaro.org> wrote: > > 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)
Hi Peter, Jason, On 28/2/25 18:47, Peter Maydell 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. > > 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 Since Jason just sent his network pull request, I'll take these patches via my hw-misc tree (with patch #2 fixed up), except if one of you object. Thanks, Phil.
On Tue, Mar 11, 2025 at 5:00 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > Hi Peter, Jason, > > On 28/2/25 18:47, Peter Maydell 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. > > > > 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 > > Since Jason just sent his network pull request, I'll take these > patches via my hw-misc tree (with patch #2 fixed up), except if > one of you object. > > Thanks, > > Phil. > Fine with me. Thanks