Re: [bug report] drm/amdgpu/gfx7: set USER_SHADER_ARRAY_CONFIG based on disable_cu parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux