The _DSM handler for the internal brightness state load command walks
the returned package and copies every element into
ctrl->backLightData without checking the object type, the element type,
or the size of the destination. A firmware-provided _DSM method can
return a non-package object, non-integer elements, or more data than
the 4 KiB backLightData array holds, causing out-of-bounds writes in
the RM control buffer.
Move the _DSM handling into helpers that validate the package and every
element, and that refuse to copy past the end of the destination. The
destination size is only updated after the whole package has been
validated, and success is only reported to the RM control command in
that case. Use a zeroed buffer for the _DSM input and reject an
invalid initial data size before subtracting it from the destination
length.
Fixes: 9e99444490238 ("drm/nouveau/disp/r535: initial support")
Signed-off-by: Kang Chen <[email protected]>
---
.../nouveau/nvkm/subdev/gsp/rm/r535/disp.c | 142 ++++++++++++------
1 file changed, 92 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
index f5f22173fc2c..730bd08da911 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/disp.c
@@ -1467,6 +1467,94 @@ r535_disp_get_static_info(struct nvkm_disp *disp)
return 0;
}
+#if defined(CONFIG_ACPI) && defined(CONFIG_X86)
+static int
+r535_gsp_backlight_pkg(union acpi_object *obj, u8 *data, u16 *data_size,
+ size_t data_len)
+{
+ size_t offset = *data_size;
+ unsigned int i;
+
+ if (offset > data_len)
+ return -EINVAL;
+
+ if (obj->type != ACPI_TYPE_PACKAGE)
+ return -EINVAL;
+
+ for (i = 0; i < obj->package.count; i++) {
+ union acpi_object *elt = &obj->package.elements[i];
+ u32 size;
+
+ if (elt->type != ACPI_TYPE_INTEGER)
+ return -EINVAL;
+
+ if (elt->integer.value & ~0xffffffffULL)
+ size = 8;
+ else
+ size = 4;
+
+ if (size > data_len - offset)
+ return -EINVAL;
+
+ memcpy(&data[offset], &elt->integer.value, size);
+ offset += size;
+ }
+
+ *data_size = offset;
+
+ return 0;
+}
+
+static bool
+r535_gsp_backlight_dsm(struct nvkm_device *device, u8 *data, u16 *data_size,
+ size_t data_len)
+{
+ const guid_t nbci_dsm_guid =
+ GUID_INIT(0xD4A50B75, 0x65C7, 0x46F7,
+ 0xBF, 0xB7, 0x41, 0x51, 0x4C, 0xEA, 0x02, 0x44);
+ const guid_t nvhg_dsm_guid =
+ GUID_INIT(0x9D95A0A0, 0x0060, 0x4D48,
+ 0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12, 0x9F, 0xD4);
+ acpi_handle handle = ACPI_HANDLE(device->dev);
+ union acpi_object argv4, *obj;
+ bool ret = false;
+ bool nbci, nvhg;
+
+ if (!handle || !acpi_has_method(handle, "_DSM"))
+ return false;
+
+ nbci = acpi_check_dsm(handle, &nbci_dsm_guid, 0x00000102,
+ 1ULL << 0x00000014);
+ nvhg = acpi_check_dsm(handle, &nvhg_dsm_guid, 0x00000102,
+ 1ULL << 0x00000014);
+ if (!nbci && !nvhg)
+ return false;
+
+ argv4.buffer.type = ACPI_TYPE_BUFFER;
+ argv4.buffer.length = data_len;
+ argv4.buffer.pointer = kzalloc(data_len, GFP_KERNEL);
+ if (!argv4.buffer.pointer)
+ return false;
+
+ obj = acpi_evaluate_dsm(handle, nbci ? &nbci_dsm_guid : &nvhg_dsm_guid,
+ 0x00000102, 0x14, &argv4);
+ if (!obj) {
+ acpi_handle_info(handle, "failed to evaluate _DSM\n");
+ goto out_free;
+ }
+
+ if (r535_gsp_backlight_pkg(obj, data, data_size, data_len))
+ acpi_handle_info(handle, "invalid _DSM response\n");
+ else
+ ret = true;
+
+ ACPI_FREE(obj);
+out_free:
+ kfree(argv4.buffer.pointer);
+ return ret;
+}
+#endif
+
static int
r535_disp_oneinit(struct nvkm_disp *disp)
{
@@ -1528,56 +1616,10 @@ r535_disp_oneinit(struct nvkm_disp *disp)
ctrl->status = 0x56; /* NV_ERR_NOT_SUPPORTED */
- {
- const guid_t NBCI_DSM_GUID =
- GUID_INIT(0xD4A50B75, 0x65C7, 0x46F7,
- 0xBF, 0xB7, 0x41, 0x51, 0x4C, 0xEA,
0x02, 0x44);
- u64 NBCI_DSM_REV = 0x00000102;
- const guid_t NVHG_DSM_GUID =
- GUID_INIT(0x9D95A0A0, 0x0060, 0x4D48,
- 0xB3, 0x4D, 0x7E, 0x5F, 0xEA, 0x12,
0x9F, 0xD4);
- u64 NVHG_DSM_REV = 0x00000102;
- acpi_handle handle = ACPI_HANDLE(device->dev);
-
- if (handle && acpi_has_method(handle, "_DSM")) {
- bool nbci = acpi_check_dsm(handle,
&NBCI_DSM_GUID, NBCI_DSM_REV,
- 1ULL << 0x00000014);
- bool nvhg = acpi_check_dsm(handle,
&NVHG_DSM_GUID, NVHG_DSM_REV,
- 1ULL << 0x00000014);
-
- if (nbci || nvhg) {
- union acpi_object argv4 = {
- .buffer.type =
ACPI_TYPE_BUFFER,
- .buffer.length =
sizeof(ctrl->backLightData),
- .buffer.pointer =
kmalloc(argv4.buffer.length, GFP_KERNEL),
- }, *obj;
-
- obj = acpi_evaluate_dsm(handle, nbci ?
&NBCI_DSM_GUID : &NVHG_DSM_GUID,
- 0x00000102,
0x14, &argv4);
- if (!obj) {
- acpi_handle_info(handle,
"failed to evaluate _DSM\n");
- } else {
- for (int i = 0; i <
obj->package.count; i++) {
- union acpi_object *elt
= &obj->package.elements[i];
- u32 size;
-
- if (elt->integer.value
& ~0xffffffffULL)
- size = 8;
- else
- size = 4;
-
-
memcpy(&ctrl->backLightData[ctrl->backLightDataSize], &elt->integer.value,
size);
- ctrl->backLightDataSize
+= size;
- }
-
- ctrl->status = 0;
- ACPI_FREE(obj);
- }
-
- kfree(argv4.buffer.pointer);
- }
- }
- }
+ if (r535_gsp_backlight_dsm(device, ctrl->backLightData,
+ &ctrl->backLightDataSize,
+ sizeof(ctrl->backLightData)))
+ ctrl->status = 0;
ret = nvkm_gsp_rm_ctrl_wr(subdevice, ctrl);
if (ret)
--
2.34.1
