Thanks! I can review the patch. On Mon, Feb 24, 2025 at 12:52 PM Pierrick Bouvier < pierrick.bouv...@linaro.org> wrote:
> Hello, > > This patch introduces a buffer-overflow, now reported by address sanitizer. > > I sent a patch: > https://lore.kernel.org/qemu-devel/20250224205053.104959-1- > pierrick.bouv...@linaro.org/T/#u > > You're welcome to review it, or fix the problem differently if there is > a better approach. > > Regards, > Pierrick > > On 2/19/25 10:45, Hao Wu wrote: > > These 2 values are different between NPCM7XX and NPCM8XX > > GCRs. So we add them to the class and assign different values > > to them. > > > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > > Signed-off-by: Hao Wu <wuhao...@google.com> > > --- > > hw/misc/npcm_gcr.c | 27 ++++++++++++++++----------- > > include/hw/misc/npcm_gcr.h | 13 +++++++++++-- > > 2 files changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/hw/misc/npcm_gcr.c b/hw/misc/npcm_gcr.c > > index 0959f2e5c4..d89e8c2c3b 100644 > > --- a/hw/misc/npcm_gcr.c > > +++ b/hw/misc/npcm_gcr.c > > @@ -66,10 +66,9 @@ enum NPCM7xxGCRRegisters { > > NPCM7XX_GCR_SCRPAD = 0x013c / sizeof(uint32_t), > > NPCM7XX_GCR_USB1PHYCTL, > > NPCM7XX_GCR_USB2PHYCTL, > > - NPCM7XX_GCR_REGS_END, > > }; > > > > -static const uint32_t cold_reset_values[NPCM7XX_GCR_NR_REGS] = { > > +static const uint32_t npcm7xx_cold_reset_values[NPCM7XX_GCR_NR_REGS] = { > > [NPCM7XX_GCR_PDID] = 0x04a92750, /* Poleg A1 */ > > [NPCM7XX_GCR_MISCPE] = 0x0000ffff, > > [NPCM7XX_GCR_SPSWC] = 0x00000003, > > @@ -88,8 +87,9 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr > offset, unsigned size) > > { > > uint32_t reg = offset / sizeof(uint32_t); > > NPCMGCRState *s = opaque; > > + NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s); > > > > - if (reg >= NPCM7XX_GCR_NR_REGS) { > > + if (reg >= c->nr_regs) { > > qemu_log_mask(LOG_GUEST_ERROR, > > "%s: offset 0x%04" HWADDR_PRIx " out of range\n", > > __func__, offset); > > @@ -106,11 +106,12 @@ static void npcm_gcr_write(void *opaque, hwaddr > offset, > > { > > uint32_t reg = offset / sizeof(uint32_t); > > NPCMGCRState *s = opaque; > > + NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s); > > uint32_t value = v; > > > > - trace_npcm_gcr_write(offset, value); > > + trace_npcm_gcr_write(offset, v); > > > > - if (reg >= NPCM7XX_GCR_NR_REGS) { > > + if (reg >= c->nr_regs) { > > qemu_log_mask(LOG_GUEST_ERROR, > > "%s: offset 0x%04" HWADDR_PRIx " out of range\n", > > __func__, offset); > > @@ -156,10 +157,12 @@ static const struct MemoryRegionOps npcm_gcr_ops = > { > > static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type) > > { > > NPCMGCRState *s = NPCM_GCR(obj); > > + NPCMGCRClass *c = NPCM_GCR_GET_CLASS(obj); > > > > - QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values)); > > - > > - memcpy(s->regs, cold_reset_values, sizeof(s->regs)); > > + g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values)); > > + g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t)); > > + memcpy(s->regs, c->cold_reset_values, c->nr_regs * > sizeof(uint32_t)); > > + /* These 3 registers are at the same location in both 7xx and 8xx. > */ > > s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron; > > s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr; > > s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3; > > @@ -224,7 +227,7 @@ static const VMStateDescription vmstate_npcm_gcr = { > > .version_id = 1, > > .minimum_version_id = 1, > > .fields = (const VMStateField[]) { > > - VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM7XX_GCR_NR_REGS), > > + VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM_GCR_MAX_NR_REGS), > > VMSTATE_END_OF_LIST(), > > }, > > }; > > @@ -238,7 +241,6 @@ static void npcm_gcr_class_init(ObjectClass *klass, > void *data) > > { > > DeviceClass *dc = DEVICE_CLASS(klass); > > > > - QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END > NPCM7XX_GCR_NR_REGS); > > dc->realize = npcm_gcr_realize; > > dc->vmsd = &vmstate_npcm_gcr; > > > > @@ -247,13 +249,15 @@ static void npcm_gcr_class_init(ObjectClass > *klass, void *data) > > > > static void npcm7xx_gcr_class_init(ObjectClass *klass, void *data) > > { > > + NPCMGCRClass *c = NPCM_GCR_CLASS(klass); > > DeviceClass *dc = DEVICE_CLASS(klass); > > ResettableClass *rc = RESETTABLE_CLASS(klass); > > > > - QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END != NPCM7XX_GCR_NR_REGS); > > dc->desc = "NPCM7xx System Global Control Registers"; > > rc->phases.enter = npcm7xx_gcr_enter_reset; > > > > + c->nr_regs = NPCM7XX_GCR_NR_REGS; > > + c->cold_reset_values = npcm7xx_cold_reset_values; > > } > > > > static const TypeInfo npcm_gcr_info[] = { > > @@ -262,6 +266,7 @@ static const TypeInfo npcm_gcr_info[] = { > > .parent = TYPE_SYS_BUS_DEVICE, > > .instance_size = sizeof(NPCMGCRState), > > .instance_init = npcm_gcr_init, > > + .class_size = sizeof(NPCMGCRClass), > > .class_init = npcm_gcr_class_init, > > .abstract = true, > > }, > > diff --git a/include/hw/misc/npcm_gcr.h b/include/hw/misc/npcm_gcr.h > > index 6d3d00d260..9af24e5cdc 100644 > > --- a/include/hw/misc/npcm_gcr.h > > +++ b/include/hw/misc/npcm_gcr.h > > @@ -18,6 +18,7 @@ > > > > #include "exec/memory.h" > > #include "hw/sysbus.h" > > +#include "qom/object.h" > > > > /* > > * NPCM7XX PWRON STRAP bit fields > > @@ -53,6 +54,7 @@ > > * Number of registers in our device state structure. Don't change > this without > > * incrementing the version_id in the vmstate. > > */ > > +#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS > > #define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t)) > > > > typedef struct NPCMGCRState { > > @@ -60,15 +62,22 @@ typedef struct NPCMGCRState { > > > > MemoryRegion iomem; > > > > - uint32_t regs[NPCM7XX_GCR_NR_REGS]; > > + uint32_t regs[NPCM_GCR_MAX_NR_REGS]; > > > > uint32_t reset_pwron; > > uint32_t reset_mdlr; > > uint32_t reset_intcr3; > > } NPCMGCRState; > > > > +typedef struct NPCMGCRClass { > > + SysBusDeviceClass parent; > > + > > + size_t nr_regs; > > + const uint32_t *cold_reset_values; > > +} NPCMGCRClass; > > + > > #define TYPE_NPCM_GCR "npcm-gcr" > > #define TYPE_NPCM7XX_GCR "npcm7xx-gcr" > > -OBJECT_DECLARE_SIMPLE_TYPE(NPCMGCRState, NPCM_GCR) > > +OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR) > > > > #endif /* NPCM_GCR_H */ > >
