⬢ DragonFlyBSD Kernel Audit
DF-4337 / fix.diff
← back to finding ↓ download raw
--- a/sys/dev/drm/radeon/atom.h
+++ b/sys/dev/drm/radeon/atom.h
@@ -127,6 +127,7 @@
 	struct lock mutex;
 	struct lock scratch_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;
 
@@ -144,7 +145,7 @@
 
 extern int atom_debug;
 
-struct atom_context *atom_parse(struct card_info *, void *);
+struct atom_context *atom_parse(struct card_info *, void *, int);
 int atom_execute_table(struct atom_context *, int, uint32_t *);
 int atom_execute_table_scratch_unlocked(struct atom_context *, int, uint32_t *);
 int atom_asic_init(struct atom_context *);
--- a/sys/dev/drm/radeon/atom.c
+++ b/sys/dev/drm/radeon/atom.c
@@ -99,6 +99,16 @@
 #define ATOM_SDEBUG_PRINT(...) do { } while (0)
 #endif
 
+/* All VBIOS offsets must be validated against the length of the bios
+ * allocation (atom_context.bios_size).  A VBIOS image self-declares its
+ * size (bios[2]*512) 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 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)
 {
@@ -276,7 +286,11 @@
 			else
 				ATOM_DEBUG_PRINT("ID[0x%04X]", idx);
 		}
-		val = U32(idx + gctx->data_block);
+		if (!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);
@@ -630,7 +644,8 @@
 		ATOM_SDEBUG_PRINT("   table: %d (%s)\n", idx, atom_table_names[idx]);
 	else
 		ATOM_SDEBUG_PRINT("   table: %d\n", idx);
-	if (U16(ctx->ctx->cmd_table + 4 + 2 * idx))
+	if (atom_in_bios(ctx->ctx, ctx->ctx->cmd_table + 4 + 2 * idx, 2) &&
+	    U16(ctx->ctx->cmd_table + 4 + 2 * idx))
 		r = atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift);
 	if (r) {
 		ctx->abort = true;
@@ -844,8 +859,10 @@
 		ctx->ctx->data_block = 0;
 	else if (idx == 255)
 		ctx->ctx->data_block = ctx->start;
-	else
+	else if (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;
 	ATOM_SDEBUG_PRINT("   base: 0x%04X\n", ctx->ctx->data_block);
 }
 
@@ -1171,6 +1188,9 @@
 	if (!base)
 		return -EINVAL;
 
+	if (!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;
@@ -1191,6 +1211,14 @@
 
 	debug_depth++;
 	while (1) {
+		/* the execution pointer is attacker-controlled VBIOS data
+		 * (jumps, switch walks); never let it leave the image */
+		if (!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)
 			ATOM_SDEBUG_PRINT("%s @ 0x%04X\n", atom_op_names[op], ptr - 1);
@@ -1258,29 +1286,41 @@
 	ctx->iio = kzalloc(2 * 256, GFP_KERNEL);
 	if (!ctx->iio)
 		return;
-	while (CU8(base) == ATOM_IIO_START) {
+	while (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 (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 *atom_parse(struct card_info *card, void *bios)
+struct atom_context *atom_parse(struct card_info *card, void *bios, int bios_size)
 {
 	int base;
 	struct atom_context *ctx =
 	    kzalloc(sizeof(struct atom_context), GFP_KERNEL);
 	char *str;
 	char name[512];
-	int i;
+	int i, iio_base;
 
 	if (!ctx)
 		return NULL;
 
 	ctx->card = card;
 	ctx->bios = bios;
+	ctx->bios_size = bios_size;
+
+	if (bios_size < 0x50) {
+		DRM_INFO("Invalid BIOS size.\n");
+		kfree(ctx);
+		return NULL;
+	}
 
 	if (CU16(0) != ATOM_BIOS_MAGIC) {
 		DRM_INFO("Invalid BIOS magic.\n");
@@ -1304,15 +1344,42 @@
 		return NULL;
 	}
 
+	if (!atom_in_bios(ctx, base, ATOM_ROM_DATA_PTR + 2)) {
+		DRM_INFO("ATOM ROM table beyond BIOS image.\n");
+		kfree(ctx);
+		return NULL;
+	}
+
 	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 (!atom_in_bios(ctx, ctx->cmd_table, 4 + 2 * 256) ||
+	    !atom_in_bios(ctx, ctx->data_table, 4 + 2 * 256) ||
+	    !atom_in_bios(ctx, ctx->data_table + ATOM_DATA_IIO_PTR, 2)) {
+		DRM_INFO("Invalid ATOM table pointers.\n");
+		kfree(ctx);
+		return NULL;
+	}
+	iio_base = CU16(ctx->data_table + ATOM_DATA_IIO_PTR) + 4;
+	if (!atom_in_bios(ctx, iio_base, 2)) {
+		DRM_INFO("Invalid ATOM IIO table pointer.\n");
+		kfree(ctx);
+		return NULL;
+	}
+	atom_index_iio(ctx, iio_base);
 	if (!ctx->iio) {
 		atom_destroy(ctx);
 		return NULL;
 	}
 
 	str = CSTR(CU16(base + ATOM_ROM_MSG_PTR));
+	if (!atom_in_bios(ctx, CU16(base + ATOM_ROM_MSG_PTR), 1)) {
+		DRM_INFO("Invalid ATOM message pointer.\n");
+		kfree(ctx->iio);
+		kfree(ctx);
+		return NULL;
+	}
 	while (*str && ((*str == '\n') || (*str == '\r')))
 		str++;
 	/* name string isn't always 0 terminated */
@@ -1335,6 +1402,9 @@
 	uint32_t ps[16];
 	int ret;
 
+	if (!atom_in_bios(ctx, hwi, 0x28))
+		return 1;
+
 	memset(ps, 0, 64);
 
 	ps[0] = cpu_to_le32(CU32(hwi + ATOM_FWI_DEFSCLK_PTR));
@@ -1368,12 +1438,19 @@
 			    uint16_t * data_start)
 {
 	int offset = index * 2 + 4;
-	int idx = CU16(ctx->data_table + offset);
 	u16 *mdt = (u16 *)((char *)ctx->bios + ctx->data_table + 4);
+	int idx;
 
+	if (index < 0 || !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 (!atom_in_bios(ctx, idx, 4) || !atom_in_bios(ctx, idx, CU16(idx)))
+		return false;
 	if (size)
 		*size = CU16(idx);
 	if (frev)
@@ -1388,12 +1465,17 @@
 			   uint8_t * crev)
 {
 	int offset = index * 2 + 4;
-	int idx = CU16(ctx->cmd_table + offset);
 	u16 *mct = (u16 *)((char *)ctx->bios + ctx->cmd_table + 4);
+	int idx;
 
+	if (index < 0 || !atom_in_bios(ctx, ctx->cmd_table + offset, 2))
+		return false;
+	idx = CU16(ctx->cmd_table + offset);
 	if (!mct[index])
 		return false;
 
+	if (!atom_in_bios(ctx, idx, 4))
+		return false;
 	if (frev)
 		*frev = CU8(idx + 2);
 	if (crev)
@@ -1411,6 +1493,9 @@
 	if (atom_parse_data_header(ctx, index, NULL, NULL, NULL, &data_offset)) {
 		firmware_usage = (struct _ATOM_VRAM_USAGE_BY_FIRMWARE *)((char *)ctx->bios + data_offset);
 
+		if (!atom_in_bios(ctx, data_offset, sizeof(*firmware_usage)))
+			return -EINVAL;
+
 		DRM_DEBUG("atom firmware requested %08x %dkb\n",
 			  le32_to_cpu(firmware_usage->asFirmwareVramReserveInfo[0].ulStartAddrUsedByFirmware),
 			  le16_to_cpu(firmware_usage->asFirmwareVramReserveInfo[0].usFirmwareUseInKb));
--- a/sys/dev/drm/radeon/radeon.h
+++ b/sys/dev/drm/radeon/radeon.h
@@ -2331,6 +2331,7 @@
 	int				disp_priority;
 	/* BIOS */
 	uint8_t				*bios;
+	int				bios_size;
 	bool				is_atom_bios;
 	uint16_t			bios_header_start;
 	struct radeon_bo		*stolen_vga_memory;
--- a/sys/dev/drm/radeon/radeon_device.c
+++ b/sys/dev/drm/radeon/radeon_device.c
@@ -984,7 +984,8 @@
 	atom_card_info->pll_read = cail_pll_read;
 	atom_card_info->pll_write = cail_pll_write;
 
-	rdev->mode_info.atom_context = atom_parse(atom_card_info, rdev->bios);
+	rdev->mode_info.atom_context = atom_parse(atom_card_info, rdev->bios,
+						  rdev->bios_size);
 	if (!rdev->mode_info.atom_context) {
 		radeon_atombios_fini(rdev);
 		return -ENOMEM;
--- a/sys/dev/drm/radeon/radeon_bios.c
+++ b/sys/dev/drm/radeon/radeon_bios.c
@@ -71,6 +71,7 @@
 		return false;
 	}
 	memcpy_fromio(rdev->bios, bios, size);
+	rdev->bios_size = size;
 	iounmap(bios);
 	return true;
 }
@@ -101,6 +102,7 @@
 		return false;
 	}
 	memcpy_fromio(rdev->bios, bios, size);
+	rdev->bios_size = size;
 	vga_pci_unmap_bios(vga_dev, bios);
 	return true;
 }
@@ -156,6 +158,7 @@
 
 	if (rdev->bios == NULL)
 		return false;
+	rdev->bios_size = size;
 
 	DRM_INFO("radeon: VBIOS read from PCI ROM BAR at 0x%08lx (%zu bytes)\n",
 	    (unsigned long)rom_addr, size);
@@ -271,6 +274,7 @@
 		DRM_ERROR("Unable to allocate bios\n");
 		return false;
 	}
+	rdev->bios_size = size;
 
 	for (i = 0; i < size / ATRM_BIOS_PAGE; i++) {
 		DRM_INFO("%s: Call radeon_atrm_call()\n", __func__);
@@ -717,6 +721,8 @@
 	rdev->bios = kmalloc(vhdr->ImageLength, M_DRM, M_WAITOK);
 	memcpy(rdev->bios, &vbios->VbiosContent, vhdr->ImageLength);
 	ret = !!rdev->bios;
+	if (ret)
+		rdev->bios_size = vhdr->ImageLength;
 
 out_unmap:
 	return ret;
@@ -747,6 +753,7 @@
 	if (r == false || rdev->bios == NULL) {
 		DRM_ERROR("Unable to locate a BIOS ROM\n");
 		rdev->bios = NULL;
+		rdev->bios_size = 0;
 		return false;
 	}
 	if (rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa) {
@@ -777,5 +784,6 @@
 free_bios:
 	kfree(rdev->bios);
 	rdev->bios = NULL;
+	rdev->bios_size = 0;
 	return false;
 }