| Message ID | 20241212114554.517379-1-gerben@altlinux.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/display: refine upper limit for offset value in assert check | expand |
On 12/12/24 05:45, gerben@altlinux.org wrote: > From: Denis Rastyogin <gerben@altlinux.org> > > Accessing an element of the s->core_registers array > with a size of 236 (0x3AC) may lead to a buffer overflow, > as the index 'offset' can exceed the valid range and reach values > up to 5139 (0x504C >> 2). This change addresses > a potential vulnerability when writing data. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Reported-by: David Meliksetyan <d.meliksetyan@fobos-nt.ru> > Signed-off-by: Denis Rastyogin <gerben@altlinux.org> > --- > hw/display/xlnx_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c > index 6ab2335499..69ccc7ccc2 100644 > --- a/hw/display/xlnx_dp.c > +++ b/hw/display/xlnx_dp.c > @@ -743,6 +743,7 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value, > DPRINTF("core write @%" PRIx64 " = 0x%8.8" PRIX64 "\n", offset, value); > > offset = offset >> 2; > + assert(offset <= (0x3AC >> 2)); > > switch (offset) { > /* > @@ -896,7 +897,6 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value, > xlnx_dp_update_irq(s); > break; > default: > - assert(offset <= (0x504C >> 2)); > s->core_registers[offset] = value; > break; > } Why are you moving the assert? The switch statement takes care of sorting non-default values of offset. More correct would be to use DP_CORE_REG_ARRAY_SIZE in the assert, along with a comment that the io region has been sized exactly to fit core_registers[]. r~
diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index 6ab2335499..69ccc7ccc2 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -743,6 +743,7 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value, DPRINTF("core write @%" PRIx64 " = 0x%8.8" PRIX64 "\n", offset, value); offset = offset >> 2; + assert(offset <= (0x3AC >> 2)); switch (offset) { /* @@ -896,7 +897,6 @@ static void xlnx_dp_write(void *opaque, hwaddr offset, uint64_t value, xlnx_dp_update_irq(s); break; default: - assert(offset <= (0x504C >> 2)); s->core_registers[offset] = value; break; }