DF-4494 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 | --- a/sys/dev/drm/radeon/rs780_dpm.c +++ b/sys/dev/drm/radeon/rs780_dpm.c @@ -600,6 +600,11 @@ struct radeon_ps *boot_ps = rdev->pm.dpm.boot_ps; int ret; + if (boot_ps == NULL) { + DRM_ERROR("no boot power state, refusing to enable DPM\n"); + return -EINVAL; + } + rs780_get_pm_mode_parameters(rdev); rs780_disable_vbios_powersaving(rdev); @@ -786,6 +791,15 @@ } } +/* DF-4492: every pplib walk below is driven by VBIOS-supplied offsets, + * indices and strides; none of them were bounds-checked, so a crafted + * PowerPlayInfo table reads past the kmalloc'd BIOS image. Bound each + * walk by the table size reported by the ATOM data-table header. */ +static bool rs780_pplib_walk_ok(u16 data_size, u32 rel_end) +{ + return rel_end <= (u32)data_size; +} + static int rs780_parse_power_table(struct radeon_device *rdev) { struct radeon_mode_info *mode_info = &rdev->mode_info; @@ -796,10 +810,11 @@ union power_info *power_info; int index = GetIndexIntoMasterTable(DATA, PowerPlayInfo); u16 data_offset; + u16 data_size; u8 frev, crev; struct igp_ps *ps; - if (!atom_parse_data_header(mode_info->atom_context, index, NULL, + if (!atom_parse_data_header(mode_info->atom_context, index, &data_size, &frev, &crev, &data_offset)) return -EINVAL; power_info = (union power_info *)((uint8_t*)mode_info->atom_context->bios + data_offset); @@ -808,18 +823,50 @@ power_info->pplib.ucNumStates, GFP_KERNEL); if (!rdev->pm.dpm.ps) return -ENOMEM; + if (power_info->pplib.ucStateEntrySize < 2) { + /* ATOM_PPLIB_STATE is 2 bytes; a smaller stride parses no + * state at all while num_ps is still set (DF-4493) */ + DRM_ERROR("invalid state entry size %d\n", + power_info->pplib.ucStateEntrySize); + goto err_free_ps; + } + if (!rs780_pplib_walk_ok(data_size, + (u32)le16_to_cpu(power_info->pplib.usStateArrayOffset) + + (u32)power_info->pplib.ucNumStates * + power_info->pplib.ucStateEntrySize)) { + DRM_ERROR("state array walks past the power table\n"); + goto err_free_ps; + } for (i = 0; i < power_info->pplib.ucNumStates; i++) { power_state = (union pplib_power_state *) ((uint8_t*)mode_info->atom_context->bios + data_offset + le16_to_cpu(power_info->pplib.usStateArrayOffset) + i * power_info->pplib.ucStateEntrySize); + if (!rs780_pplib_walk_ok(data_size, + (u32)le16_to_cpu(power_info->pplib.usNonClockInfoArrayOffset) + + (u32)power_state->v1.ucNonClockStateIndex * + power_info->pplib.ucNonClockSize + + sizeof(struct _ATOM_PPLIB_NONCLOCK_INFO))) { + DRM_ERROR("nonClockInfoIndex %d walks past the table\n", + power_state->v1.ucNonClockStateIndex); + goto err_free_ps; + } non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *) ((uint8_t*)mode_info->atom_context->bios + data_offset + le16_to_cpu(power_info->pplib.usNonClockInfoArrayOffset) + (power_state->v1.ucNonClockStateIndex * power_info->pplib.ucNonClockSize)); if (power_info->pplib.ucStateEntrySize - 1) { + if (!rs780_pplib_walk_ok(data_size, + (u32)le16_to_cpu(power_info->pplib.usClockInfoArrayOffset) + + (u32)power_state->v1.ucClockStateIndices[0] * + power_info->pplib.ucClockInfoSize + + sizeof(union pplib_clock_info))) { + DRM_ERROR("clockInfoIndex %d walks past the table\n", + power_state->v1.ucClockStateIndices[0]); + goto err_free_ps; + } clock_info = (union pplib_clock_info *) ((uint8_t*)mode_info->atom_context->bios + data_offset + le16_to_cpu(power_info->pplib.usClockInfoArrayOffset) + @@ -827,7 +874,12 @@ power_info->pplib.ucClockInfoSize)); ps = kzalloc(sizeof(struct igp_ps), GFP_KERNEL); if (ps == NULL) { + while (i--) + kfree(rdev->pm.dpm.ps[i].ps_priv); kfree(rdev->pm.dpm.ps); + rdev->pm.dpm.ps = NULL; + rdev->pm.dpm.boot_ps = NULL; + rdev->pm.dpm.uvd_ps = NULL; return -ENOMEM; } rdev->pm.dpm.ps[i].ps_priv = ps; @@ -841,6 +893,13 @@ } rdev->pm.dpm.num_ps = power_info->pplib.ucNumStates; return 0; + +err_free_ps: + kfree(rdev->pm.dpm.ps); + rdev->pm.dpm.ps = NULL; + rdev->pm.dpm.boot_ps = NULL; + rdev->pm.dpm.uvd_ps = NULL; + return -EINVAL; } int rs780_dpm_init(struct radeon_device *rdev) @@ -930,7 +989,10 @@ return 0; } - radeon_dpm_fini(rdev); + /* DF-4494: do NOT tear the state down here. radeon_pm_init_dpm + * (radeon_pm.c:1448) discards this error and keeps going, so every + * pointer must stay valid (or NULL) until radeon_pm_fini owns the + * teardown exactly once. */ return -EINVAL; } |