| Message ID | 20240723131029.1159908-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/misc/bcm2835_property: Fix set-palette, OTP-access properties | expand |
On 23/7/24 15:10, Peter Maydell wrote: > The documentation of the "Set palette" mailbox property at > https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface#set-palette > says it has the form: > > Length: 24..1032 > Value: > u32: offset: first palette index to set (0-255) > u32: length: number of palette entries to set (1-256) > u32...: RGBA palette values (offset to offset+length-1) > > We get this wrong in a couple of ways: > * we aren't checking the offset and length are in range, so the guest > can make us spin for a long time by providing a large length > * the bounds check on our loop is wrong: we should iterate through > 'length' palette entries, not 'length - offset' entries > > Fix the loop to implement the bounds checks and get the loop > condition right. In the process, make the variables local to > this switch case, rather than function-global, so it's clearer > what type they are when reading the code. > > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/misc/bcm2835_property.c | 27 ++++++++++++++++----------- > 1 file changed, 16 insertions(+), 11 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c index 63de3db6215..e28fdca9846 100644 --- a/hw/misc/bcm2835_property.c +++ b/hw/misc/bcm2835_property.c @@ -31,7 +31,6 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) size_t resplen; uint32_t tmp; int n; - uint32_t offset, length, color; uint32_t start_num, number, otp_row; /* @@ -274,19 +273,25 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value) resplen = 16; break; case RPI_FWREQ_FRAMEBUFFER_SET_PALETTE: - offset = ldl_le_phys(&s->dma_as, value + 12); - length = ldl_le_phys(&s->dma_as, value + 16); - n = 0; - while (n < length - offset) { - color = ldl_le_phys(&s->dma_as, value + 20 + (n << 2)); - stl_le_phys(&s->dma_as, - s->fbdev->vcram_base + ((offset + n) << 2), color); - n++; + { + uint32_t offset = ldl_le_phys(&s->dma_as, value + 12); + uint32_t length = ldl_le_phys(&s->dma_as, value + 16); + int resp; + + if (offset > 255 || length < 1 || length > 256) { + resp = 1; /* invalid request */ + } else { + for (uint32_t e = 0; e < length; e++) { + uint32_t color = ldl_le_phys(&s->dma_as, value + 20 + (e << 2)); + stl_le_phys(&s->dma_as, + s->fbdev->vcram_base + ((offset + e) << 2), color); + } + resp = 0; } - stl_le_phys(&s->dma_as, value + 12, 0); + stl_le_phys(&s->dma_as, value + 12, resp); resplen = 4; break; - + } case RPI_FWREQ_FRAMEBUFFER_GET_NUM_DISPLAYS: stl_le_phys(&s->dma_as, value + 12, 1); resplen = 4;
The documentation of the "Set palette" mailbox property at https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface#set-palette says it has the form: Length: 24..1032 Value: u32: offset: first palette index to set (0-255) u32: length: number of palette entries to set (1-256) u32...: RGBA palette values (offset to offset+length-1) We get this wrong in a couple of ways: * we aren't checking the offset and length are in range, so the guest can make us spin for a long time by providing a large length * the bounds check on our loop is wrong: we should iterate through 'length' palette entries, not 'length - offset' entries Fix the loop to implement the bounds checks and get the loop condition right. In the process, make the variables local to this switch case, rather than function-global, so it's clearer what type they are when reading the code. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/bcm2835_property.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)