The ATOM interpreter executes bytecode straight out of the VBIOS image and multiple cursor-mutation paths are not bounded against the image size:
- the main opcode fetch loop follows jump/skip targets anywhere (jump targets are u16 offsets that can point past a small image), - atom_op_switch scans for ATOM_CASE_END with no limit; a table that omits the marker walks off the allocated BIOS copy, - atom_op_processds advances the cursor by an unchecked u16 taken from the table itself, - the command-table base offset from the master table is used unchecked before its header fields are read. On a malformed/malicious VBIOS image, these reads could run past the kmalloc'd BIOS buffer and the out-of-bounds bytes get interpreted as further opcodes/operands. atom_context::bios_size and the corresponding amdgpu_atom_parse() parameter already exist in this tree; this uses that existing infrastructure to bound the four sites above. atom_index_iio() is already bounds-checked separately and is not touched here. Assisted-by: gkh_clanker:t1000 Signed-off-by: Hari Mishal <[email protected]> --- drivers/gpu/drm/amd/amdgpu/atom.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c index e0e585f280e2..56c8a17edea1 100644 --- a/drivers/gpu/drm/amd/amdgpu/atom.c +++ b/drivers/gpu/drm/amd/amdgpu/atom.c @@ -1032,8 +1032,12 @@ static void atom_op_switch(atom_exec_context *ctx, int *ptr, int arg) uint32_t src, val, target; SDEBUG(" switch: "); src = atom_get_src(ctx, attr, ptr); - while (U16(*ptr) != ATOM_CASE_END) + while ((uint32_t)*ptr + 2 <= ctx->ctx->bios_size && + U16(*ptr) != ATOM_CASE_END) if (U8(*ptr) == ATOM_CASE_MAGIC) { + /* magic byte + immediate (up to 4) + 2-byte target */ + if ((uint32_t)*ptr + 7 > ctx->ctx->bios_size) + break; (*ptr)++; SDEBUG(" case: "); val = @@ -1087,7 +1091,14 @@ static void atom_op_debug(atom_exec_context *ctx, int *ptr, int arg) static void atom_op_processds(atom_exec_context *ctx, int *ptr, int arg) { - uint16_t val = U16(*ptr); + u16 val; + + /* the skip count is bytecode-supplied; validate before reading it */ + if ((uint32_t)*ptr + 2 > ctx->ctx->bios_size) { + ctx->abort = true; + return; + } + val = U16(*ptr); (*ptr) += val + 2; SDEBUG("PROCESSDS output: 0x%02X\n", val); } @@ -1237,6 +1248,13 @@ static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, if (!base) return -EINVAL; + /* the table offset itself comes from the VBIOS master table */ + if ((uint32_t)base + ATOM_CT_CODE_PTR >= ctx->bios_size) { + DRM_ERROR("atombios command table %d offset 0x%x outside VBIOS image\n", + index, base); + return -EINVAL; + } + if (ctx->execute_depth >= ATOM_EXECUTE_MAX_DEPTH) { DRM_ERROR("atombios command table nesting exceeded limit (%u)\n", ATOM_EXECUTE_MAX_DEPTH); @@ -1273,6 +1291,13 @@ static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index, debug_depth++; while (1) { + /* jump/skip targets are bytecode-supplied; stay inside the image */ + if ((uint32_t)ptr >= ctx->bios_size) { + DRM_ERROR("atombios cursor 0x%x outside VBIOS image (table %04X)\n", + ptr, base); + ret = -EINVAL; + goto free; + } op = CU8(ptr++); if (op < ATOM_OP_NAMES_CNT) SDEBUG("%s @ 0x%04X\n", atom_op_names[op], ptr - 1); -- 2.43.0
