[bug report] drm/amd/amdgpu: Add debugfs support for reading GPRs (v2)

Dan Carpenter dan.carpenter at oracle.com
Tue Nov 28 14:29:19 UTC 2017


Hello Tom St Denis,

The patch c5a60ce81b49: "drm/amd/amdgpu: Add debugfs support for
reading GPRs (v2)" from Dec 5, 2016, leads to the following static
checker warning:

	drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3774 amdgpu_debugfs_gpr_read()
	error: buffer overflow 'data' 1024 <= 4095

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
  3731  static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
  3732                                          size_t size, loff_t *pos)
  3733  {
  3734          struct amdgpu_device *adev = f->f_inode->i_private;
  3735          int r;
  3736          ssize_t result = 0;
  3737          uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data;
  3738  
  3739          if (size & 3 || *pos & 3)
  3740                  return -EINVAL;
  3741  
  3742          /* decode offset */
  3743          offset = *pos & GENMASK_ULL(11, 0);
                ^^^^^^
offset is set to 0-4095.

  3744          se = (*pos & GENMASK_ULL(19, 12)) >> 12;
  3745          sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
  3746          cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
  3747          wave = (*pos & GENMASK_ULL(43, 36)) >> 36;
  3748          simd = (*pos & GENMASK_ULL(51, 44)) >> 44;
  3749          thread = (*pos & GENMASK_ULL(59, 52)) >> 52;
  3750          bank = (*pos & GENMASK_ULL(61, 60)) >> 60;
  3751  
  3752          data = kmalloc_array(1024, sizeof(*data), GFP_KERNEL);
                                     ^^^^
data is a 1024 element array

  3753          if (!data)
  3754                  return -ENOMEM;
  3755  
  3756          /* switch to the specific se/sh/cu */
  3757          mutex_lock(&adev->grbm_idx_mutex);
  3758          amdgpu_gfx_select_se_sh(adev, se, sh, cu);
  3759  
  3760          if (bank == 0) {
  3761                  if (adev->gfx.funcs->read_wave_vgprs)
  3762                          adev->gfx.funcs->read_wave_vgprs(adev, simd, wave, thread, offset, size>>2, data);
  3763          } else {
  3764                  if (adev->gfx.funcs->read_wave_sgprs)
  3765                          adev->gfx.funcs->read_wave_sgprs(adev, simd, wave, offset, size>>2, data);
  3766          }
  3767  
  3768          amdgpu_gfx_select_se_sh(adev, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF);
  3769          mutex_unlock(&adev->grbm_idx_mutex);
  3770  
  3771          while (size) {
  3772                  uint32_t value;
  3773  
  3774                  value = data[offset++];
                                ^^^^^^^^^^^^^^
We're possibly reading beyond the end of the array.  Maybe we are
supposed to divide the offset /= sizeof(*data)?

  3775                  r = put_user(value, (uint32_t *)buf);

regards,
dan carpenter


More information about the amd-gfx mailing list