On 28/2/25 20:16, Peter Maydell wrote:
For accesses to the 91c111 data register, the address within the packet's data frame is determined by a combination of the pointer register and the offset used to access the data register, so that you can access data at effectively wider than byte width. The pointer register's pointer field is 11 bits wide, which is exactly the size to index a 2048-byte data frame.We weren't quite getting the logic right for ensuring that we end up with a pointer value to use in the s->data[][] array that isn't out of bounds: * we correctly mask when getting the initial pointer value * for the "autoincrement the pointer register" case, we correctly mask after adding 1 so that the pointer register wraps back around at the 2048 byte mark * but for the non-autoincrement case where we have to add the low 2 bits of the data register offset, we don't account for the possibility that the pointer register is 0x7ff and the addition should wrap Fix this bug by factoring out the "get the p value to use as an array index" into a function, making it use FIELD macro names rather than hard-coded constants, and having a utility function that does "add a value and wrap it" that we can use both for the "autoincrement" and "add the offset bits" codepaths. Cc: qemu-sta...@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2758 Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> --- Based-on: 20250228174802.1945417-1-peter.mayd...@linaro.org ("hw/net/smc91c111: Fix potential array overflows") I'd only just sent the other smc91c111 patchset when I found another fuzzer bug for it in the bug tracker :-) The "modernisation" of the code seemed clearer to me than doing a minimal in-place fix (which would look something like "p = (p + (offset & 3) & 0x7ff;" in both read and write functions). But it does make the actual bugfix a little less obvious. --- hw/net/smc91c111.c | 65 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 12 deletions(-)
Well clean, thanks. Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
