Skip to content

Commit

Permalink
drm/radeon: check PS, WS index
Browse files Browse the repository at this point in the history 
Theoretically, it would be possible for a buggy or malicious VBIOS to
overwrite past the bounds of the passed parameters (or its own
workspace); add bounds checking to prevent this from happening.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3093
Signed-off-by: Alexander Richards <electrodeyt@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
  • Loading branch information
Alexander Richards authored and Alex Deucher committed Jan 15, 2024
1 parent 4630d50 commit f7a16fa
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 74 deletions.
47 changes: 33 additions & 14 deletions drivers/gpu/drm/radeon/atom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
typedef struct {
struct atom_context *ctx;
uint32_t *ps, *ws;
int ps_size, ws_size;
int ps_shift;
uint16_t start;
unsigned last_jump;
Expand All @@ -68,8 +69,8 @@ typedef struct {
} atom_exec_context;

int atom_debug = 0;
static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t *params);
int atom_execute_table(struct atom_context *ctx, int index, uint32_t *params);
static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t *params, int params_size);
int atom_execute_table(struct atom_context *ctx, int index, uint32_t *params, int params_size);

static uint32_t atom_arg_mask[8] = {
0xFFFFFFFF, 0x0000FFFF, 0x00FFFF00, 0xFFFF0000,
Expand Down Expand Up @@ -221,7 +222,10 @@ static uint32_t atom_get_src_int(atom_exec_context *ctx, uint8_t attr,
(*ptr)++;
/* get_unaligned_le32 avoids unaligned accesses from atombios
* tables, noticed on a DEC Alpha. */
val = get_unaligned_le32((u32 *)&ctx->ps[idx]);
if (idx < ctx->ps_size)
val = get_unaligned_le32((u32 *)&ctx->ps[idx]);
else
pr_info("PS index out of range: %i > %i\n", idx, ctx->ps_size);
if (print)
DEBUG("PS[0x%02X,0x%04X]", idx, val);
break;
Expand Down Expand Up @@ -259,7 +263,10 @@ static uint32_t atom_get_src_int(atom_exec_context *ctx, uint8_t attr,
val = gctx->reg_block;
break;
default:
val = ctx->ws[idx];
if (idx < ctx->ws_size)
val = ctx->ws[idx];
else
pr_info("WS index out of range: %i > %i\n", idx, ctx->ws_size);
}
break;
case ATOM_ARG_ID:
Expand Down Expand Up @@ -494,6 +501,10 @@ static void atom_put_dst(atom_exec_context *ctx, int arg, uint8_t attr,
idx = U8(*ptr);
(*ptr)++;
DEBUG("PS[0x%02X]", idx);
if (idx >= ctx->ps_size) {
pr_info("PS index out of range: %i > %i\n", idx, ctx->ps_size);
return;
}
ctx->ps[idx] = cpu_to_le32(val);
break;
case ATOM_ARG_WS:
Expand Down Expand Up @@ -526,6 +537,10 @@ static void atom_put_dst(atom_exec_context *ctx, int arg, uint8_t attr,
gctx->reg_block = val;
break;
default:
if (idx >= ctx->ws_size) {
pr_info("WS index out of range: %i > %i\n", idx, ctx->ws_size);
return;
}
ctx->ws[idx] = val;
}
break;
Expand Down Expand Up @@ -623,7 +638,7 @@ static void atom_op_calltable(atom_exec_context *ctx, int *ptr, int arg)
else
SDEBUG(" table: %d\n", idx);
if (U16(ctx->ctx->cmd_table + 4 + 2 * idx))
r = atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift);
r = atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
if (r) {
ctx->abort = true;
}
Expand Down Expand Up @@ -1152,7 +1167,7 @@ static struct {
atom_op_shr, ATOM_ARG_MC}, {
atom_op_debug, 0},};

static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t *params)
static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32_t *params, int params_size)
{
int base = CU16(ctx->cmd_table + 4 + 2 * index);
int len, ws, ps, ptr;
Expand All @@ -1174,12 +1189,16 @@ static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32
ectx.ps_shift = ps / 4;
ectx.start = base;
ectx.ps = params;
ectx.ps_size = params_size;
ectx.abort = false;
ectx.last_jump = 0;
if (ws)
if (ws) {
ectx.ws = kcalloc(4, ws, GFP_KERNEL);
else
ectx.ws_size = ws;
} else {
ectx.ws = NULL;
ectx.ws_size = 0;
}

debug_depth++;
while (1) {
Expand Down Expand Up @@ -1212,7 +1231,7 @@ static int atom_execute_table_locked(struct atom_context *ctx, int index, uint32
return ret;
}

int atom_execute_table_scratch_unlocked(struct atom_context *ctx, int index, uint32_t *params)
int atom_execute_table_scratch_unlocked(struct atom_context *ctx, int index, uint32_t *params, int params_size)
{
int r;

Expand All @@ -1228,16 +1247,16 @@ int atom_execute_table_scratch_unlocked(struct atom_context *ctx, int index, uin
/* reset divmul */
ctx->divmul[0] = 0;
ctx->divmul[1] = 0;
r = atom_execute_table_locked(ctx, index, params);
r = atom_execute_table_locked(ctx, index, params, params_size);
mutex_unlock(&ctx->mutex);
return r;
}

int atom_execute_table(struct atom_context *ctx, int index, uint32_t *params)
int atom_execute_table(struct atom_context *ctx, int index, uint32_t *params, int params_size)
{
int r;
mutex_lock(&ctx->scratch_mutex);
r = atom_execute_table_scratch_unlocked(ctx, index, params);
r = atom_execute_table_scratch_unlocked(ctx, index, params, params_size);
mutex_unlock(&ctx->scratch_mutex);
return r;
}
Expand Down Expand Up @@ -1335,15 +1354,15 @@ int atom_asic_init(struct atom_context *ctx)

if (!CU16(ctx->cmd_table + 4 + 2 * ATOM_CMD_INIT))
return 1;
ret = atom_execute_table(ctx, ATOM_CMD_INIT, ps);
ret = atom_execute_table(ctx, ATOM_CMD_INIT, ps, 16);
if (ret)
return ret;

memset(ps, 0, 64);

if (rdev->family < CHIP_R600) {
if (CU16(ctx->cmd_table + 4 + 2 * ATOM_CMD_SPDFANCNTL))
atom_execute_table(ctx, ATOM_CMD_SPDFANCNTL, ps);
atom_execute_table(ctx, ATOM_CMD_SPDFANCNTL, ps, 16);
}
return ret;
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/radeon/atom.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ struct atom_context {
extern int atom_debug;

struct atom_context *atom_parse(struct card_info *, void *);
int atom_execute_table(struct atom_context *, int, uint32_t *);
int atom_execute_table_scratch_unlocked(struct atom_context *, int, uint32_t *);
int atom_execute_table(struct atom_context *, int, uint32_t *, int);
int atom_execute_table_scratch_unlocked(struct atom_context *, int, uint32_t *, int);
int atom_asic_init(struct atom_context *);
void atom_destroy(struct atom_context *);
bool atom_parse_data_header(struct atom_context *ctx, int index, uint16_t *size,
Expand Down
28 changes: 14 additions & 14 deletions drivers/gpu/drm/radeon/atombios_crtc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ static void atombios_overscan_setup(struct drm_crtc *crtc,
args.usOverscanTop = cpu_to_le16(radeon_crtc->v_border);
break;
}
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_scaler_setup(struct drm_crtc *crtc)
Expand Down Expand Up @@ -157,7 +157,7 @@ static void atombios_scaler_setup(struct drm_crtc *crtc)
break;
}
}
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
if ((is_tv || is_cv)
&& rdev->family >= CHIP_RV515 && rdev->family <= CHIP_R580) {
atom_rv515_force_tv_scaler(rdev, radeon_crtc);
Expand All @@ -178,7 +178,7 @@ static void atombios_lock_crtc(struct drm_crtc *crtc, int lock)
args.ucCRTC = radeon_crtc->crtc_id;
args.ucEnable = lock;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_enable_crtc(struct drm_crtc *crtc, int state)
Expand All @@ -194,7 +194,7 @@ static void atombios_enable_crtc(struct drm_crtc *crtc, int state)
args.ucCRTC = radeon_crtc->crtc_id;
args.ucEnable = state;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_enable_crtc_memreq(struct drm_crtc *crtc, int state)
Expand All @@ -210,7 +210,7 @@ static void atombios_enable_crtc_memreq(struct drm_crtc *crtc, int state)
args.ucCRTC = radeon_crtc->crtc_id;
args.ucEnable = state;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static const u32 vga_control_regs[6] =
Expand Down Expand Up @@ -242,7 +242,7 @@ static void atombios_blank_crtc(struct drm_crtc *crtc, int state)
args.ucCRTC = radeon_crtc->crtc_id;
args.ucBlanking = state;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));

if (ASIC_IS_DCE8(rdev))
WREG32(vga_control_regs[radeon_crtc->crtc_id], vga_control);
Expand All @@ -261,7 +261,7 @@ static void atombios_powergate_crtc(struct drm_crtc *crtc, int state)
args.ucDispPipeId = radeon_crtc->crtc_id;
args.ucEnable = state;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

void atombios_crtc_dpms(struct drm_crtc *crtc, int mode)
Expand Down Expand Up @@ -343,7 +343,7 @@ atombios_set_crtc_dtd_timing(struct drm_crtc *crtc,
args.susModeMiscInfo.usAccess = cpu_to_le16(misc);
args.ucCRTC = radeon_crtc->crtc_id;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_crtc_set_timing(struct drm_crtc *crtc,
Expand Down Expand Up @@ -389,7 +389,7 @@ static void atombios_crtc_set_timing(struct drm_crtc *crtc,
args.susModeMiscInfo.usAccess = cpu_to_le16(misc);
args.ucCRTC = radeon_crtc->crtc_id;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_disable_ss(struct radeon_device *rdev, int pll_id)
Expand Down Expand Up @@ -546,7 +546,7 @@ static void atombios_crtc_program_ss(struct radeon_device *rdev,
args.lvds_ss.ucSpreadSpectrumStepSize_Delay |= (ss->delay & 7) << 4;
args.lvds_ss.ucEnable = enable;
}
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

union adjust_pixel_clock {
Expand Down Expand Up @@ -692,7 +692,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
ADJUST_DISPLAY_CONFIG_SS_ENABLE;

atom_execute_table(rdev->mode_info.atom_context,
index, (uint32_t *)&args);
index, (uint32_t *)&args, sizeof(args));
adjusted_clock = le16_to_cpu(args.v1.usPixelClock) * 10;
break;
case 3:
Expand Down Expand Up @@ -725,7 +725,7 @@ static u32 atombios_adjust_pll(struct drm_crtc *crtc,
args.v3.sInput.ucExtTransmitterID = 0;

atom_execute_table(rdev->mode_info.atom_context,
index, (uint32_t *)&args);
index, (uint32_t *)&args, sizeof(args));
adjusted_clock = le32_to_cpu(args.v3.sOutput.ulDispPllFreq) * 10;
if (args.v3.sOutput.ucRefDiv) {
radeon_crtc->pll_flags |= RADEON_PLL_USE_FRAC_FB_DIV;
Expand Down Expand Up @@ -809,7 +809,7 @@ static void atombios_crtc_set_disp_eng_pll(struct radeon_device *rdev,
DRM_ERROR("Unknown table version %d %d\n", frev, crev);
return;
}
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static void atombios_crtc_program_pll(struct drm_crtc *crtc,
Expand Down Expand Up @@ -949,7 +949,7 @@ static void atombios_crtc_program_pll(struct drm_crtc *crtc,
return;
}

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
}

static bool atombios_crtc_prepare_pll(struct drm_crtc *crtc, struct drm_display_mode *mode)
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/radeon/atombios_dp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static int radeon_process_aux_ch(struct radeon_i2c_chan *chan,
if (ASIC_IS_DCE4(rdev))
args.v2.ucHPD_ID = chan->rec.hpd;

atom_execute_table_scratch_unlocked(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table_scratch_unlocked(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));

*ack = args.v1.ucReplyStatus;

Expand Down Expand Up @@ -354,7 +354,7 @@ static u8 radeon_dp_encoder_service(struct radeon_device *rdev,
args.ucLaneNum = lane_num;
args.ucStatus = 0;

atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args);
atom_execute_table(rdev->mode_info.atom_context, index, (uint32_t *)&args, sizeof(args));
return args.ucStatus;
}

Expand Down
Loading

0 comments on commit f7a16fa

Please sign in to comment.