Re: [PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer

2023-11-24 Thread kernel test robot
Hi Lu,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.7-rc2 next-20231124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Lu-Yao/drm-amdgpu-Fix-cat-debugfs-amdgpu_regs_didt-causes-kernel-null-pointer/20231122-203138
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:
https://lore.kernel.org/r/20231122093509.34302-1-yaolu%40kylinos.cn
patch subject: [PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes 
kernel null pointer
config: x86_64-randconfig-001-20231123 
(https://download.01.org/0day-ci/archive/20231124/202311241442.f0s4bazk-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git 
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20231124/202311241442.f0s4bazk-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202311241442.f0s4bazk-...@intel.com/

All errors (new ones prefixed by >>):

   warning: unknown warning option '-Wstringop-truncation'; did you mean 
'-Wstring-concatenation'? [-Wunknown-warning-option]
   warning: unknown warning option '-Wpacked-not-aligned'; did you mean 
'-Wpacked-non-pod'? [-Wunknown-warning-option]
>> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:642:55: error: use of undeclared 
>> identifier '__FUNC__'
   dev_err(adev->dev, "%s adev->didt_rreg is null!\n", 
__FUNC__);
   ^
   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c:703:55: error: use of undeclared 
identifier '__FUNC__'
   dev_err(adev->dev, "%s adev->didt_wreg is null!\n", 
__FUNC__);
   ^
   2 warnings and 2 errors generated.


vim +/__FUNC__ +642 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c

   618  
   619  /**
   620   * amdgpu_debugfs_regs_didt_read - Read from a DIDT register
   621   *
   622   * @f: open file handle
   623   * @buf: User buffer to store read data in
   624   * @size: Number of bytes to read
   625   * @pos:  Offset to seek to
   626   *
   627   * The lower bits are the BYTE offset of the register to read.  This
   628   * allows reading multiple registers in a single call and having
   629   * the returned size reflect that.
   630   */
   631  static ssize_t amdgpu_debugfs_regs_didt_read(struct file *f, char 
__user *buf,
   632  size_t size, loff_t *pos)
   633  {
   634  struct amdgpu_device *adev = file_inode(f)->i_private;
   635  ssize_t result = 0;
   636  int r;
   637  
   638  if (size & 0x3 || *pos & 0x3)
   639  return -EINVAL;
   640  
   641  if (adev->didt_rreg == NULL) {
 > 642  dev_err(adev->dev, "%s adev->didt_rreg is null!\n", 
 > __FUNC__);
   643  return -EPERM;
   644  }
   645  
   646  r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
   647  if (r < 0) {
   648  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   649  return r;
   650  }
   651  
   652  r = amdgpu_virt_enable_access_debugfs(adev);
   653  if (r < 0) {
   654  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   655  return r;
   656  }
   657  
   658  while (size) {
   659  uint32_t value;
   660  
   661  value = RREG32_DIDT(*pos >> 2);
   662  r = put_user(value, (uint32_t *)buf);
   663  if (r)
   664  goto out;
   665  
   666  result += 4;
   667  buf += 4;
   668  *pos += 4;
   669  size -= 4;
   670  }
   671  
   672  r = result;
   673  out:
   674  pm_runtime_mark_last_busy(adev_to_drm(adev)->dev);
   675  pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
   676  amdgpu_virt_disable_access_debugfs(adev);
   677  return r;
   678  }
   679  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


[PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer

2023-11-22 Thread Lu Yao
For 'AMDGPU_FAMILY_SI' family cards, in 'si_common_early_init' func, init
'didt_rreg' and 'didt_wreg' to 'NULL'. But in func
'amdgpu_debugfs_regs_didt_read/write', using 'RREG32_DIDT' 'WREG32_DIDT'
lacks of relevant judgment. And other 'amdgpu_ip_block_version' that use
these two definitions won't be added for 'AMDGPU_FAMILY_SI'.

So, add null pointer judgment before calling.

Signed-off-by: Lu Yao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index a53f436fa9f1..797d7d3bfd50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -638,6 +638,11 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file 
*f, char __user *buf,
if (size & 0x3 || *pos & 0x3)
return -EINVAL;
 
+   if (adev->didt_rreg == NULL) {
+   dev_err(adev->dev, "%s adev->didt_rreg is null!\n", __FUNC__);
+   return -EPERM;
+   }
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -694,6 +699,11 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file 
*f, const char __user
if (size & 0x3 || *pos & 0x3)
return -EINVAL;
 
+   if (adev->didt_wreg == NULL) {
+   dev_err(adev->dev, "%s adev->didt_wreg is null!\n", __FUNC__);
+   return -EPERM;
+   }
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
-- 
2.25.1



Re: [PATCH] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer

2023-11-22 Thread Christian König

Am 22.11.23 um 10:35 schrieb Lu Yao:

For 'AMDGPU_FAMILY_SI' family cards, in 'si_common_early_init' func, init
'didt_rreg' and 'didt_wreg' to 'NULL'. But in func
'amdgpu_debugfs_regs_didt_read/write', using 'RREG32_DIDT' 'WREG32_DIDT'
lacks of relevant judgment. And other 'amdgpu_ip_block_version' that use
these two definitions won't be added for 'AMDGPU_FAMILY_SI'.

So, add null pointer judgment before calling.

Signed-off-by: Lu Yao 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index a53f436fa9f1..797d7d3bfd50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -638,6 +638,11 @@ static ssize_t amdgpu_debugfs_regs_didt_read(struct file 
*f, char __user *buf,
if (size & 0x3 || *pos & 0x3)
return -EINVAL;
  
+	if (adev->didt_rreg == NULL) {

+   dev_err(adev->dev, "%s adev->didt_rreg is null!\n", __FUNC__);


Please drop the dev_err(), this is not a device error but rather 
userspace using a functionality not applicable for this device type.



+   return -EPERM;


Maybe rather use EOPNOTSUPP here.

Regards,
Christian.


+   }
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -694,6 +699,11 @@ static ssize_t amdgpu_debugfs_regs_didt_write(struct file 
*f, const char __user
if (size & 0x3 || *pos & 0x3)
return -EINVAL;
  
+	if (adev->didt_wreg == NULL) {

+   dev_err(adev->dev, "%s adev->didt_wreg is null!\n", __FUNC__);
+   return -EPERM;
+   }
+
r = pm_runtime_get_sync(adev_to_drm(adev)->dev);
if (r < 0) {
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);