| Message ID | 20231123012234.5783-1-yaolu@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/amdgpu: Fix cat debugfs amdgpu_regs_didt causes kernel null pointer | expand |
Am 23.11.23 um 02:22 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 <yaolu@kylinos.cn> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > Changes in v2: > 1. Drop dev_err message. > 2. Change error code from 'EPERM' to 'EOPNOTSUPP' > Link to v1: https://lore.kernel.org/all/20231122093509.34302-1-yaolu@kylinos.cn/ > Thanks Christian for his comments. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > index a53f436fa9f1..e098cd66fa2a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > @@ -638,6 +638,9 @@ 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) > + return -EOPNOTSUPP; > + > r = pm_runtime_get_sync(adev_to_drm(adev)->dev); > if (r < 0) { > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > @@ -694,6 +697,9 @@ 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) > + return -EOPNOTSUPP; > + > r = pm_runtime_get_sync(adev_to_drm(adev)->dev); > if (r < 0) { > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
Applied. Thanks! Alex On Thu, Nov 23, 2023 at 3:22 AM Christian König <christian.koenig@amd.com> wrote: > > Am 23.11.23 um 02:22 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 <yaolu@kylinos.cn> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > > --- > > Changes in v2: > > 1. Drop dev_err message. > > 2. Change error code from 'EPERM' to 'EOPNOTSUPP' > > Link to v1: https://lore.kernel.org/all/20231122093509.34302-1-yaolu@kylinos.cn/ > > Thanks Christian for his comments. > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > index a53f436fa9f1..e098cd66fa2a 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c > > @@ -638,6 +638,9 @@ 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) > > + return -EOPNOTSUPP; > > + > > r = pm_runtime_get_sync(adev_to_drm(adev)->dev); > > if (r < 0) { > > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); > > @@ -694,6 +697,9 @@ 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) > > + return -EOPNOTSUPP; > > + > > r = pm_runtime_get_sync(adev_to_drm(adev)->dev); > > if (r < 0) { > > pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c index a53f436fa9f1..e098cd66fa2a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c @@ -638,6 +638,9 @@ 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) + return -EOPNOTSUPP; + r = pm_runtime_get_sync(adev_to_drm(adev)->dev); if (r < 0) { pm_runtime_put_autosuspend(adev_to_drm(adev)->dev); @@ -694,6 +697,9 @@ 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) + return -EOPNOTSUPP; + r = pm_runtime_get_sync(adev_to_drm(adev)->dev); if (r < 0) { pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
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 <yaolu@kylinos.cn> --- Changes in v2: 1. Drop dev_err message. 2. Change error code from 'EPERM' to 'EOPNOTSUPP' Link to v1: https://lore.kernel.org/all/20231122093509.34302-1-yaolu@kylinos.cn/ Thanks Christian for his comments. --- drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 6 ++++++ 1 file changed, 6 insertions(+)