MXMS data can be provided by the ROM, DSM or WMI paths. The parser only kept the data pointer, so later validation and descriptor iteration had no record of the actual provider buffer length. The ROM path also read a short temporary header before deriving the declared MXMS size.
Keep the provider buffer size with the MXMS data, fetch the complete fixed header before reading the declared structure length, and validate each MXMS descriptor header and record array against the remaining MXMS buffer before dumping or handing the descriptor to consumers. Signed-off-by: Pengpeng Hou <[email protected]> --- .../gpu/drm/nouveau/nvkm/subdev/mxm/base.c | 24 ++++++++-- .../gpu/drm/nouveau/nvkm/subdev/mxm/mxms.c | 47 +++++++++++++++---- .../gpu/drm/nouveau/nvkm/subdev/mxm/priv.h | 1 + 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c index 26ec50b66e66..ad74a216a201 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/base.c @@ -47,7 +47,8 @@ mxm_shadow_rom(struct nvkm_mxm *mxm, u8 version) struct nvkm_bios *bios = device->bios; struct nvkm_i2c *i2c = device->i2c; struct nvkm_i2c_bus *bus = NULL; - u8 i2cidx, mxms[6], addr, size; + u8 i2cidx, mxms[8], addr; + u32 size; i2cidx = mxm_ddc_map(bios, 1 /* LVDS_DDC */) & 0x0f; if (i2cidx < 0x0f) @@ -56,22 +57,32 @@ mxm_shadow_rom(struct nvkm_mxm *mxm, u8 version) return false; addr = 0x54; - if (!mxm_shadow_rom_fetch(bus, addr, 0, 6, mxms)) { + if (!mxm_shadow_rom_fetch(bus, addr, 0, sizeof(mxms), mxms)) { addr = 0x56; - if (!mxm_shadow_rom_fetch(bus, addr, 0, 6, mxms)) + if (!mxm_shadow_rom_fetch(bus, addr, 0, sizeof(mxms), mxms)) return false; } mxm->mxms = mxms; + mxm->mxms_size = sizeof(mxms); size = mxms_headerlen(mxm) + mxms_structlen(mxm); + if (size < mxms_headerlen(mxm) || size > U8_MAX) { + mxm->mxms = NULL; + mxm->mxms_size = 0; + return false; + } + mxm->mxms = kmalloc(size, GFP_KERNEL); if (mxm->mxms && - mxm_shadow_rom_fetch(bus, addr, 0, size, mxm->mxms)) + mxm_shadow_rom_fetch(bus, addr, 0, size, mxm->mxms)) { + mxm->mxms_size = size; return true; + } kfree(mxm->mxms); mxm->mxms = NULL; + mxm->mxms_size = 0; return false; } @@ -113,6 +124,8 @@ mxm_shadow_dsm(struct nvkm_mxm *mxm, u8 version) if (obj->type == ACPI_TYPE_BUFFER) { mxm->mxms = kmemdup(obj->buffer.pointer, obj->buffer.length, GFP_KERNEL); + if (mxm->mxms) + mxm->mxms_size = obj->buffer.length; } else if (obj->type == ACPI_TYPE_INTEGER) { nvkm_debug(subdev, "DSM MXMS returned 0x%llx\n", obj->integer.value); @@ -188,6 +201,8 @@ mxm_shadow_wmi(struct nvkm_mxm *mxm, u8 version) if (obj->type == ACPI_TYPE_BUFFER) { mxm->mxms = kmemdup(obj->buffer.pointer, obj->buffer.length, GFP_KERNEL); + if (mxm->mxms) + mxm->mxms_size = obj->buffer.length; } kfree(obj); @@ -220,6 +235,7 @@ mxm_shadow(struct nvkm_mxm *mxm, u8 version) return 0; kfree(mxm->mxms); mxm->mxms = NULL; + mxm->mxms_size = 0; } } while ((++shadow)->name); return -ENOENT; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/mxms.c b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/mxms.c index 9abfa5e2fe9f..895a1d76b80f 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/mxms.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/mxms.c @@ -60,14 +60,21 @@ mxms_headerlen(struct nvkm_mxm *mxm) u16 mxms_structlen(struct nvkm_mxm *mxm) { - return *(u16 *)&mxms_data(mxm)[6]; + if (mxm->mxms_size < mxms_headerlen(mxm)) + return 0; + + return ROM16(mxms_data(mxm)[6]); } bool mxms_checksum(struct nvkm_mxm *mxm) { - u16 size = mxms_headerlen(mxm) + mxms_structlen(mxm); + u32 size = mxms_headerlen(mxm) + mxms_structlen(mxm); u8 *mxms = mxms_data(mxm), sum = 0; + + if (size < mxms_headerlen(mxm) || mxm->mxms_size < size) + return false; + while (size--) sum += *mxms++; if (sum) { @@ -81,7 +88,13 @@ bool mxms_valid(struct nvkm_mxm *mxm) { u8 *mxms = mxms_data(mxm); - if (*(u32 *)mxms != 0x5f4d584d) { + + if (mxm->mxms_size < mxms_headerlen(mxm)) { + nvkm_debug(&mxm->subdev, "header too short\n"); + return false; + } + + if (ROM32(mxms[0]) != 0x5f4d584d) { nvkm_debug(&mxm->subdev, "signature invalid\n"); return false; } @@ -97,15 +110,21 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, bool (*exec)(struct nvkm_mxm *, u8 *, void *), void *info) { struct nvkm_subdev *subdev = &mxm->subdev; + u32 remaining = mxms_structlen(mxm); u8 *mxms = mxms_data(mxm); u8 *desc = mxms + mxms_headerlen(mxm); - u8 *fini = desc + mxms_structlen(mxm) - 1; - while (desc < fini) { - u8 type = desc[0] & 0x0f; + + if (mxm->mxms_size < mxms_headerlen(mxm) + remaining) + return false; + + while (remaining) { + u32 desclen; + u8 type; u8 headerlen = 0; u8 recordlen = 0; u8 entries = 0; + type = desc[0] & 0x0f; switch (type) { case 0: /* Output Device Structure */ if (mxms_version(mxm) >= 0x0300) @@ -119,6 +138,8 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, headerlen = 4; break; case 4: /* GPIO Device Structure */ + if (remaining < sizeof(u32)) + return false; headerlen = 4; recordlen = 2; entries = (ROM32(desc[0]) & 0x01f00000) >> 20; @@ -128,6 +149,8 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, break; case 6: /* Backlight Control Structure */ if (mxms_version(mxm) >= 0x0300) { + if (remaining < 2) + return false; headerlen = 4; recordlen = 8; entries = (desc[1] & 0xf0) >> 4; @@ -136,6 +159,8 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, } break; case 7: /* Fan Control Structure */ + if (remaining < 2) + return false; headerlen = 8; recordlen = 4; entries = desc[1] & 0x07; @@ -145,8 +170,13 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, return false; } + if (headerlen > remaining || + (recordlen && entries > (remaining - headerlen) / recordlen)) + return false; + + desclen = headerlen + entries * recordlen; if (mxm->subdev.debug >= NV_DBG_DEBUG && (exec == NULL)) { - static const char * mxms_desc[] = { + static const char * const mxms_desc[] = { "ODS", "SCCS", "TS", "IPS", "GSD", "VSS", "BCS", "FCS", }; @@ -171,7 +201,8 @@ mxms_foreach(struct nvkm_mxm *mxm, u8 types, return false; } - desc += headerlen + (entries * recordlen); + desc += desclen; + remaining -= desclen; } return true; diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/priv.h b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/priv.h index fcacb6c6a7f7..ad93a7ceac29 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/priv.h +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/mxm/priv.h @@ -9,6 +9,7 @@ struct nvkm_mxm { struct nvkm_subdev subdev; u32 action; + u32 mxms_size; u8 *mxms; }; -- 2.43.0
