⬢ DragonFlyBSD Kernel Audit
DF-4345 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/drm/amd/amdgpu/atom.h	2026-09-19 15:19:20.795658674 +0000
+++ b/sys/dev/drm/amd/amdgpu/atom.h	2026-09-19 15:19:49.539290453 +0000
@@ -127,6 +127,7 @@
 	struct card_info *card;
 	struct lock mutex;
 	void *bios;
+	int bios_size;		/* length of the bios allocation; all bios offsets are validated against it */
 	uint32_t cmd_table, data_table;
 	uint16_t *iio;
 
@@ -145,7 +146,7 @@
 
 extern int amdgpu_atom_debug;
 
-struct atom_context *amdgpu_atom_parse(struct card_info *, void *);
+struct atom_context *amdgpu_atom_parse(struct card_info *, void *, int);
 int amdgpu_atom_execute_table(struct atom_context *, int, uint32_t *);
 int amdgpu_atom_asic_init(struct atom_context *);
 void amdgpu_atom_destroy(struct atom_context *);
--- a/sys/dev/drm/amd/amdgpu/atom.c	2026-09-19 15:19:20.795658674 +0000
+++ b/sys/dev/drm/amd/amdgpu/atom.c	2026-09-19 15:19:49.555290249 +0000
@@ -98,6 +98,17 @@
 #define SDEBUG(...) do { } while (0)
 #endif
 
+/* All VBIOS offsets must be validated against the length of the bios
+ * allocation (atom_context.bios_size, plumbed from the amdgpu_bios.c
+ * readers via adev->bios_size).  A VBIOS image self-declares its size
+ * and that may be far smaller than the 16-bit offset space the ATOM
+ * tables use, so every dereference of a bios-derived offset is an
+ * out-of-bounds read unless it passes this check. */
+static bool amdgpu_atom_in_bios(const struct atom_context *ctx, int off, int len)
+{
+	return off >= 0 && len >= 0 && off <= ctx->bios_size - len;
+}
+
 static uint32_t atom_iio_execute(struct atom_context *ctx, int base,
 				 uint32_t index, uint32_t data)
 {
@@ -265,7 +276,11 @@
 			else
 				DEBUG("ID[0x%04X]", idx);
 		}
-		val = U32(idx + gctx->data_block);
+		if (!amdgpu_atom_in_bios(gctx, idx + gctx->data_block, 4)) {
+			DRM_ERROR("ATOM: ID operand beyond BIOS image\n");
+			val = 0;
+		} else
+			val = U32(idx + gctx->data_block);
 		break;
 	case ATOM_ARG_FB:
 		idx = U8(*ptr);
@@ -616,7 +631,8 @@
 		SDEBUG("   table: %d (%s)\n", idx, atom_table_names[idx]);
 	else
 		SDEBUG("   table: %d\n", idx);
-	if (U16(ctx->ctx->cmd_table + 4 + 2 * idx))
+	if (amdgpu_atom_in_bios(ctx->ctx, ctx->ctx->cmd_table + 4 + 2 * idx, 2) &&
+	    U16(ctx->ctx->cmd_table + 4 + 2 * idx))
 		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift);
 	if (r) {
 		ctx->abort = true;
@@ -865,8 +881,10 @@
 		ctx->ctx->data_block = 0;
 	else if (idx == 255)
 		ctx->ctx->data_block = ctx->start;
-	else
+	else if (amdgpu_atom_in_bios(ctx->ctx, ctx->ctx->data_table + 4 + 2 * idx, 2))
 		ctx->ctx->data_block = U16(ctx->ctx->data_table + 4 + 2 * idx);
+	else
+		ctx->ctx->data_block = 0;
 	SDEBUG("   base: 0x%04X\n", ctx->ctx->data_block);
 }
 
@@ -1206,6 +1224,9 @@
 	if (!base)
 		return -EINVAL;
 
+	if (!amdgpu_atom_in_bios(ctx, base, ATOM_CT_CODE_PTR + 1))
+		return -EINVAL;
+
 	len = CU16(base + ATOM_CT_SIZE_PTR);
 	ws = CU8(base + ATOM_CT_WS_PTR);
 	ps = CU8(base + ATOM_CT_PS_PTR) & ATOM_CT_PS_MASK;
@@ -1226,6 +1247,14 @@
 
 	debug_depth++;
 	while (1) {
+		/* the execution pointer is attacker-controlled VBIOS data
+		 * (jumps, switch walks); never let it leave the image */
+		if (!amdgpu_atom_in_bios(ctx, ptr, 1)) {
+			DRM_ERROR("atombios execution pointer outside BIOS image @ 0x%04X\n",
+				  ptr);
+			ret = -EINVAL;
+			goto free;
+		}
 		op = CU8(ptr++);
 		if (op < ATOM_OP_NAMES_CNT)
 			SDEBUG("%s @ 0x%04X\n", atom_op_names[op], ptr - 1);
@@ -1284,18 +1313,24 @@
 	ctx->iio = kzalloc(2 * 256, GFP_KERNEL);
 	if (!ctx->iio)
 		return;
-	while (CU8(base) == ATOM_IIO_START) {
+	while (amdgpu_atom_in_bios(ctx, base, 2) && CU8(base) == ATOM_IIO_START) {
 		ctx->iio[CU8(base + 1)] = base + 2;
 		base += 2;
-		while (CU8(base) != ATOM_IIO_END)
-			base += atom_iio_len[CU8(base)];
+		while (amdgpu_atom_in_bios(ctx, base, 1) && CU8(base) != ATOM_IIO_END) {
+			int op = CU8(base);
+
+			if (op > ATOM_IIO_END)
+				return;
+			base += atom_iio_len[op];
+		}
 		base += 3;
 	}
 }
 
-struct atom_context *amdgpu_atom_parse(struct card_info *card, void *bios)
+struct atom_context *amdgpu_atom_parse(struct card_info *card, void *bios,
+				       int bios_size)
 {
-	int base;
+	int base, iio_base;
 	struct atom_context *ctx =
 	    kzalloc(sizeof(struct atom_context), GFP_KERNEL);
 	char *str;
@@ -1306,6 +1341,13 @@
 
 	ctx->card = card;
 	ctx->bios = bios;
+	ctx->bios_size = bios_size;
+
+	if (bios_size < 0x50) {
+		pr_info("Invalid BIOS size\n");
+		kfree(ctx);
+		return NULL;
+	}
 
 	if (CU16(0) != ATOM_BIOS_MAGIC) {
 		pr_info("Invalid BIOS magic\n");
@@ -1321,6 +1363,11 @@
 	}
 
 	base = CU16(ATOM_ROM_TABLE_PTR);
+	if (!amdgpu_atom_in_bios(ctx, base, ATOM_ROM_DATA_PTR + 2)) {
+		pr_info("ATOM ROM table beyond BIOS image\n");
+		kfree(ctx);
+		return NULL;
+	}
 	if (strncmp
 	    (CSTR(base + ATOM_ROM_MAGIC_PTR), ATOM_ROM_MAGIC,
 	     strlen(ATOM_ROM_MAGIC))) {
@@ -1331,18 +1378,50 @@
 
 	ctx->cmd_table = CU16(base + ATOM_ROM_CMD_PTR);
 	ctx->data_table = CU16(base + ATOM_ROM_DATA_PTR);
-	atom_index_iio(ctx, CU16(ctx->data_table + ATOM_DATA_IIO_PTR) + 4);
+	/* the master command/data tables are arrays of up to 256 u16
+	 * offsets located at cmd_table+4 / data_table+4 */
+	if (!amdgpu_atom_in_bios(ctx, ctx->cmd_table, 4 + 2 * 256) ||
+	    !amdgpu_atom_in_bios(ctx, ctx->data_table, 4 + 2 * 256) ||
+	    !amdgpu_atom_in_bios(ctx, ctx->data_table + ATOM_DATA_IIO_PTR, 2)) {
+		pr_info("Invalid ATOM table pointers\n");
+		kfree(ctx);
+		return NULL;
+	}
+	iio_base = CU16(ctx->data_table + ATOM_DATA_IIO_PTR) + 4;
+	if (!amdgpu_atom_in_bios(ctx, iio_base, 2)) {
+		pr_info("Invalid ATOM IIO table pointer\n");
+		kfree(ctx);
+		return NULL;
+	}
+	atom_index_iio(ctx, iio_base);
 	if (!ctx->iio) {
 		amdgpu_atom_destroy(ctx);
 		return NULL;
 	}
 
-	idx = CU16(ATOM_ROM_PART_NUMBER_PTR);
-	if (idx == 0)
-		idx = 0x80;
+	if (!amdgpu_atom_in_bios(ctx, ATOM_ROM_PART_NUMBER_PTR, 2)) {
+		idx = 0;
+		str = NULL;
+	} else {
+		idx = CU16(ATOM_ROM_PART_NUMBER_PTR);
+		if (idx == 0)
+			idx = 0x80;
+		str = CSTR(idx);
+		/* the part-number string must be NUL-terminated inside
+		 * the image: %s and strlcpy walk it unbounded otherwise */
+		if (!amdgpu_atom_in_bios(ctx, idx, 1)) {
+			str = NULL;
+		} else {
+			int end = idx;
 
-	str = CSTR(idx);
-	if (*str != '\0') {
+			while (end < ctx->bios_size &&
+			       ((char *)bios)[end] != '\0')
+				end++;
+			if (end == ctx->bios_size)
+				str = NULL;
+		}
+	}
+	if (str != NULL && *str != '\0') {
 		pr_info("ATOM BIOS: %s\n", str);
 		strlcpy(ctx->vbios_version, str, sizeof(ctx->vbios_version));
 	}
@@ -1353,10 +1432,16 @@
 
 int amdgpu_atom_asic_init(struct atom_context *ctx)
 {
-	int hwi = CU16(ctx->data_table + ATOM_DATA_FWI_PTR);
+	int hwi;
 	uint32_t ps[16];
 	int ret;
 
+	if (!amdgpu_atom_in_bios(ctx, ctx->data_table + ATOM_DATA_FWI_PTR, 2))
+		return 1;
+	hwi = CU16(ctx->data_table + ATOM_DATA_FWI_PTR);
+	if (!amdgpu_atom_in_bios(ctx, hwi, 0x28))
+		return 1;
+
 	memset(ps, 0, 64);
 
 	ps[0] = cpu_to_le32(CU32(hwi + ATOM_FWI_DEFSCLK_PTR));
@@ -1364,7 +1449,8 @@
 	if (!ps[0] || !ps[1])
 		return 1;
 
-	if (!CU16(ctx->cmd_table + 4 + 2 * ATOM_CMD_INIT))
+	if (!amdgpu_atom_in_bios(ctx, ctx->cmd_table + 4 + 2 * ATOM_CMD_INIT, 2) ||
+	    !CU16(ctx->cmd_table + 4 + 2 * ATOM_CMD_INIT))
 		return 1;
 	ret = amdgpu_atom_execute_table(ctx, ATOM_CMD_INIT, ps);
 	if (ret)
@@ -1386,12 +1472,20 @@
 			    uint16_t * data_start)
 {
 	int offset = index * 2 + 4;
-	int idx = CU16(ctx->data_table + offset);
 	u16 *mdt = (u16 *)(ctx->bios + ctx->data_table + 4);
+	int idx;
 
+	if (index < 0 || !amdgpu_atom_in_bios(ctx, ctx->data_table + offset, 2))
+		return false;
+	idx = CU16(ctx->data_table + offset);
 	if (!mdt[index])
 		return false;
 
+	/* common table header is size(2) frev(1) crev(1); the table body
+	 * must lie entirely within the bios image */
+	if (!amdgpu_atom_in_bios(ctx, idx, 4) ||
+	    !amdgpu_atom_in_bios(ctx, idx, CU16(idx)))
+		return false;
 	if (size)
 		*size = CU16(idx);
 	if (frev)
@@ -1406,12 +1500,17 @@
 			   uint8_t * crev)
 {
 	int offset = index * 2 + 4;
-	int idx = CU16(ctx->cmd_table + offset);
 	u16 *mct = (u16 *)(ctx->bios + ctx->cmd_table + 4);
+	int idx;
 
+	if (index < 0 || !amdgpu_atom_in_bios(ctx, ctx->cmd_table + offset, 2))
+		return false;
+	idx = CU16(ctx->cmd_table + offset);
 	if (!mct[index])
 		return false;
 
+	if (!amdgpu_atom_in_bios(ctx, idx, 4))
+		return false;
 	if (frev)
 		*frev = CU8(idx + 2);
 	if (crev)
--- a/sys/dev/drm/amd/amdgpu/amdgpu_atombios.c	2026-09-19 15:19:20.795658674 +0000
+++ b/sys/dev/drm/amd/amdgpu/amdgpu_atombios.c	2026-09-19 15:19:49.555290249 +0000
@@ -2034,7 +2034,8 @@
 	atom_card_info->pll_read = cail_pll_read;
 	atom_card_info->pll_write = cail_pll_write;
 
-	adev->mode_info.atom_context = amdgpu_atom_parse(atom_card_info, adev->bios);
+	adev->mode_info.atom_context = amdgpu_atom_parse(atom_card_info, adev->bios,
+							adev->bios_size);
 	if (!adev->mode_info.atom_context) {
 		amdgpu_atombios_fini(adev);
 		return -ENOMEM;