--- 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; }