* [PATCH 3/3] drm/amdgpu/vi: fix buffer overflow in vi_get_register_value()
[not found] <20200825111843.GA285523@mwanda>
@ 2020-08-25 11:19 ` Dan Carpenter
0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2020-08-25 11:19 UTC (permalink / raw)
To: Alex Deucher
Cc: David Airlie, dri-devel, kernel-janitors, Wenhui Sheng, amd-gfx,
Evan Quan, Christian König, Monk Liu, Hawking Zhang
The values for "se_num" and "sh_num" come from the user in the ioctl.
They can be in the 0-255 range but if they're more than
AMDGPU_GFX_MAX_SE (4) or AMDGPU_GFX_MAX_SH_PER_SE (2) then it results in
an out of bounds read.
I split this function into to two to make the error handling simpler.
Fixes: db9635cc14f3 ("drm/amdgpu: used cached gca values for vi_read_register (v2)")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/gpu/drm/amd/amdgpu/vi.c | 197 +++++++++++++++++---------------
1 file changed, 105 insertions(+), 92 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index f6f2ed0830b1..fd623ad9d51f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -527,99 +527,108 @@ static const struct amdgpu_allowed_register_entry vi_allowed_read_registers[] =
{mmPA_SC_RASTER_CONFIG_1, true},
};
-static uint32_t vi_get_register_value(struct amdgpu_device *adev,
- bool indexed, u32 se_num,
- u32 sh_num, u32 reg_offset)
-{
- if (indexed) {
- uint32_t val;
- unsigned se_idx = (se_num == 0xffffffff) ? 0 : se_num;
- unsigned sh_idx = (sh_num == 0xffffffff) ? 0 : sh_num;
-
- switch (reg_offset) {
- case mmCC_RB_BACKEND_DISABLE:
- return adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
- case mmGC_USER_RB_BACKEND_DISABLE:
- return adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
- case mmPA_SC_RASTER_CONFIG:
- return adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
- case mmPA_SC_RASTER_CONFIG_1:
- return adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
- }
+static int vi_get_register_value_indexed(struct amdgpu_device *adev,
+ u32 se_num, u32 sh_num,
+ u32 reg_offset, u32 *value)
+{
+ unsigned se_idx = (se_num == 0xffffffff) ? 0 : se_num;
+ unsigned sh_idx = (sh_num == 0xffffffff) ? 0 : sh_num;
- mutex_lock(&adev->grbm_idx_mutex);
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff);
+ if (se_idx >= AMDGPU_GFX_MAX_SE ||
+ sh_idx >= AMDGPU_GFX_MAX_SH_PER_SE)
+ return -EINVAL;
- val = RREG32(reg_offset);
+ switch (reg_offset) {
+ case mmCC_RB_BACKEND_DISABLE:
+ *value = adev->gfx.config.rb_config[se_idx][sh_idx].rb_backend_disable;
+ return 0;
+ case mmGC_USER_RB_BACKEND_DISABLE:
+ *value = adev->gfx.config.rb_config[se_idx][sh_idx].user_rb_backend_disable;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG:
+ *value = adev->gfx.config.rb_config[se_idx][sh_idx].raster_config;
+ return 0;
+ case mmPA_SC_RASTER_CONFIG_1:
+ *value = adev->gfx.config.rb_config[se_idx][sh_idx].raster_config_1;
+ return 0;
+ }
- if (se_num != 0xffffffff || sh_num != 0xffffffff)
- amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
- mutex_unlock(&adev->grbm_idx_mutex);
- return val;
- } else {
- unsigned idx;
-
- switch (reg_offset) {
- case mmGB_ADDR_CONFIG:
- return adev->gfx.config.gb_addr_config;
- case mmMC_ARB_RAMCFG:
- return adev->gfx.config.mc_arb_ramcfg;
- case mmGB_TILE_MODE0:
- case mmGB_TILE_MODE1:
- case mmGB_TILE_MODE2:
- case mmGB_TILE_MODE3:
- case mmGB_TILE_MODE4:
- case mmGB_TILE_MODE5:
- case mmGB_TILE_MODE6:
- case mmGB_TILE_MODE7:
- case mmGB_TILE_MODE8:
- case mmGB_TILE_MODE9:
- case mmGB_TILE_MODE10:
- case mmGB_TILE_MODE11:
- case mmGB_TILE_MODE12:
- case mmGB_TILE_MODE13:
- case mmGB_TILE_MODE14:
- case mmGB_TILE_MODE15:
- case mmGB_TILE_MODE16:
- case mmGB_TILE_MODE17:
- case mmGB_TILE_MODE18:
- case mmGB_TILE_MODE19:
- case mmGB_TILE_MODE20:
- case mmGB_TILE_MODE21:
- case mmGB_TILE_MODE22:
- case mmGB_TILE_MODE23:
- case mmGB_TILE_MODE24:
- case mmGB_TILE_MODE25:
- case mmGB_TILE_MODE26:
- case mmGB_TILE_MODE27:
- case mmGB_TILE_MODE28:
- case mmGB_TILE_MODE29:
- case mmGB_TILE_MODE30:
- case mmGB_TILE_MODE31:
- idx = (reg_offset - mmGB_TILE_MODE0);
- return adev->gfx.config.tile_mode_array[idx];
- case mmGB_MACROTILE_MODE0:
- case mmGB_MACROTILE_MODE1:
- case mmGB_MACROTILE_MODE2:
- case mmGB_MACROTILE_MODE3:
- case mmGB_MACROTILE_MODE4:
- case mmGB_MACROTILE_MODE5:
- case mmGB_MACROTILE_MODE6:
- case mmGB_MACROTILE_MODE7:
- case mmGB_MACROTILE_MODE8:
- case mmGB_MACROTILE_MODE9:
- case mmGB_MACROTILE_MODE10:
- case mmGB_MACROTILE_MODE11:
- case mmGB_MACROTILE_MODE12:
- case mmGB_MACROTILE_MODE13:
- case mmGB_MACROTILE_MODE14:
- case mmGB_MACROTILE_MODE15:
- idx = (reg_offset - mmGB_MACROTILE_MODE0);
- return adev->gfx.config.macrotile_mode_array[idx];
- default:
- return RREG32(reg_offset);
- }
+ mutex_lock(&adev->grbm_idx_mutex);
+ if (se_num != 0xffffffff || sh_num != 0xffffffff)
+ amdgpu_gfx_select_se_sh(adev, se_num, sh_num, 0xffffffff);
+
+ *value = RREG32(reg_offset);
+
+ if (se_num != 0xffffffff || sh_num != 0xffffffff)
+ amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
+ mutex_unlock(&adev->grbm_idx_mutex);
+ return 0;
+}
+
+static uint32_t vi_get_register_value(struct amdgpu_device *adev,
+ u32 reg_offset)
+{
+ unsigned idx;
+
+ switch (reg_offset) {
+ case mmGB_ADDR_CONFIG:
+ return adev->gfx.config.gb_addr_config;
+ case mmMC_ARB_RAMCFG:
+ return adev->gfx.config.mc_arb_ramcfg;
+ case mmGB_TILE_MODE0:
+ case mmGB_TILE_MODE1:
+ case mmGB_TILE_MODE2:
+ case mmGB_TILE_MODE3:
+ case mmGB_TILE_MODE4:
+ case mmGB_TILE_MODE5:
+ case mmGB_TILE_MODE6:
+ case mmGB_TILE_MODE7:
+ case mmGB_TILE_MODE8:
+ case mmGB_TILE_MODE9:
+ case mmGB_TILE_MODE10:
+ case mmGB_TILE_MODE11:
+ case mmGB_TILE_MODE12:
+ case mmGB_TILE_MODE13:
+ case mmGB_TILE_MODE14:
+ case mmGB_TILE_MODE15:
+ case mmGB_TILE_MODE16:
+ case mmGB_TILE_MODE17:
+ case mmGB_TILE_MODE18:
+ case mmGB_TILE_MODE19:
+ case mmGB_TILE_MODE20:
+ case mmGB_TILE_MODE21:
+ case mmGB_TILE_MODE22:
+ case mmGB_TILE_MODE23:
+ case mmGB_TILE_MODE24:
+ case mmGB_TILE_MODE25:
+ case mmGB_TILE_MODE26:
+ case mmGB_TILE_MODE27:
+ case mmGB_TILE_MODE28:
+ case mmGB_TILE_MODE29:
+ case mmGB_TILE_MODE30:
+ case mmGB_TILE_MODE31:
+ idx = (reg_offset - mmGB_TILE_MODE0);
+ return adev->gfx.config.tile_mode_array[idx];
+ case mmGB_MACROTILE_MODE0:
+ case mmGB_MACROTILE_MODE1:
+ case mmGB_MACROTILE_MODE2:
+ case mmGB_MACROTILE_MODE3:
+ case mmGB_MACROTILE_MODE4:
+ case mmGB_MACROTILE_MODE5:
+ case mmGB_MACROTILE_MODE6:
+ case mmGB_MACROTILE_MODE7:
+ case mmGB_MACROTILE_MODE8:
+ case mmGB_MACROTILE_MODE9:
+ case mmGB_MACROTILE_MODE10:
+ case mmGB_MACROTILE_MODE11:
+ case mmGB_MACROTILE_MODE12:
+ case mmGB_MACROTILE_MODE13:
+ case mmGB_MACROTILE_MODE14:
+ case mmGB_MACROTILE_MODE15:
+ idx = (reg_offset - mmGB_MACROTILE_MODE0);
+ return adev->gfx.config.macrotile_mode_array[idx];
+ default:
+ return RREG32(reg_offset);
}
}
@@ -635,8 +644,12 @@ static int vi_read_register(struct amdgpu_device *adev, u32 se_num,
if (reg_offset != vi_allowed_read_registers[i].reg_offset)
continue;
- *value = vi_get_register_value(adev, indexed, se_num, sh_num,
- reg_offset);
+ if (indexed)
+ return vi_get_register_value_indexed(adev,
+ se_num, sh_num,
+ reg_offset, value);
+
+ *value = vi_get_register_value(adev, reg_offset);
return 0;
}
return -EINVAL;
--
2.28.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] only message in thread