[PATCH] drm/amdgpu: bound ATOM bytecode cursor within the VBIOS image

[Hari Mishal]

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 <harimishal1@xxxxxxxxx>
---
 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