On 03.08.2016 11:09, Dan Carpenter wrote:
Hello Nicolai Hähnle,
The patch 324c614a819a: "drm/amdgpu/gfx7: set USER_SHADER_ARRAY_CONFIG based on disable_cu parameter" from Jun 17, 2016, leads to the following static checker warning:
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:5057 gfx_v7_0_get_cu_info() error: buffer overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 5035 static void gfx_v7_0_get_cu_info(struct amdgpu_device *adev) 5036 { 5037 int i, j, k, counter, active_cu_number = 0; 5038 u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0; 5039 struct amdgpu_cu_info *cu_info = &adev->gfx.cu_info; 5040 unsigned disable_masks[4 * 2]; 5041 5042 memset(cu_info, 0, sizeof(*cu_info)); 5043 5044 amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2); 5045 5046 mutex_lock(&adev->grbm_idx_mutex); 5047 for (i = 0; i < adev->gfx.config.max_shader_engines; i++) { 5048 for (j = 0; j < adev->gfx.config.max_sh_per_se; j++) { 5049 mask = 1; 5050 ao_bitmap = 0; 5051 counter = 0; 5052 gfx_v7_0_select_se_sh(adev, i, j, 0xffffffff); 5053 if (i < 4 && j < 2) ^^^^^ Is it really possible for i to be >= 4?
No, because for that to happen we would have to add support for hardware with > 4 shader engines. What's the idiomatic way to express this kind of assumption in the kernel? BUG_ON(adev->gfx.config.max_shader_engines
4)? Some other form of assert?
Nicolai
5054 gfx_v7_0_set_user_cu_inactive_bitmap( 5055 adev, disable_masks[i * 2 + j]); 5056 bitmap = gfx_v7_0_get_cu_active_bitmap(adev); 5057 cu_info->bitmap[i][j] = bitmap; ^^^^^^^^^^^^^^^^^^^^^ Because if so, then we are screwed here.
5058 5059 for (k = 0; k < 16; k ++) { 5060 if (bitmap & mask) { 5061 if (counter < 2) 5062 ao_bitmap |= mask; 5063 counter ++; 5064 } 5065 mask <<= 1; 5066 } 5067 active_cu_number += counter; 5068 ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8)); 5069 } 5070 } 5071 gfx_v7_0_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff); 5072 mutex_unlock(&adev->grbm_idx_mutex); 5073 5074 cu_info->number = active_cu_number; 5075 cu_info->ao_cu_mask = ao_cu_mask; 5076 }
regards, dan carpenter