drm/amdgpu: fix AMDGPU_INFO_READ_MMR_REG

There were multiple issues in that code.

First of all the order between the reset semaphore and the mm_lock was
wrong (e.g. copy_to_user) was called while holding the lock.

Then we allocated memory while holding the reset semaphore which is also
a pretty big bug and can deadlock.

Then we used down_read_trylock() instead of waiting for the reset to
finish.

Signed-off-by: Christian König <christian.koenig@amd.com>
Fixes: 9e823f307074 ("drm/amdgpu: Block MMR_READ IOCTL in reset")
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit 361b6e6b303d4b691f6c5974d3eaab67ca6dd90e)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 06efce38f323821125b2372f78947ff02f174a30..71272f40feef8376fd4e4b3e61879d71629f6732 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -873,68 +873,59 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
                                    ? -EFAULT : 0;
        }
        case AMDGPU_INFO_READ_MMR_REG: {
-               int ret = 0;
-               unsigned int n, alloc_size;
-               uint32_t *regs;
                unsigned int se_num = (info->read_mmr_reg.instance >>
                                   AMDGPU_INFO_MMR_SE_INDEX_SHIFT) &
                                  AMDGPU_INFO_MMR_SE_INDEX_MASK;
                unsigned int sh_num = (info->read_mmr_reg.instance >>
                                   AMDGPU_INFO_MMR_SH_INDEX_SHIFT) &
                                  AMDGPU_INFO_MMR_SH_INDEX_MASK;
-
-               if (!down_read_trylock(&adev->reset_domain->sem))
-                       return -ENOENT;
+               unsigned int alloc_size;
+               uint32_t *regs;
+               int ret;
 
                /* set full masks if the userspace set all bits
                 * in the bitfields
                 */
-               if (se_num == AMDGPU_INFO_MMR_SE_INDEX_MASK) {
+               if (se_num == AMDGPU_INFO_MMR_SE_INDEX_MASK)
                        se_num = 0xffffffff;
-               } else if (se_num >= AMDGPU_GFX_MAX_SE) {
-                       ret = -EINVAL;
-                       goto out;
-               }
+               else if (se_num >= AMDGPU_GFX_MAX_SE)
+                       return -EINVAL;
 
-               if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) {
+               if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK)
                        sh_num = 0xffffffff;
-               } else if (sh_num >= AMDGPU_GFX_MAX_SH_PER_SE) {
-                       ret = -EINVAL;
-                       goto out;
-               }
+               else if (sh_num >= AMDGPU_GFX_MAX_SH_PER_SE)
+                       return -EINVAL;
 
-               if (info->read_mmr_reg.count > 128) {
-                       ret = -EINVAL;
-                       goto out;
-               }
+               if (info->read_mmr_reg.count > 128)
+                       return -EINVAL;
 
-               regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL);
-               if (!regs) {
-                       ret = -ENOMEM;
-                       goto out;
-               }
+               regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs),
+                                    GFP_KERNEL);
+               if (!regs)
+                       return -ENOMEM;
 
+               down_read(&adev->reset_domain->sem);
                alloc_size = info->read_mmr_reg.count * sizeof(*regs);
-
                amdgpu_gfx_off_ctrl(adev, false);
+               ret = 0;
                for (i = 0; i < info->read_mmr_reg.count; i++) {
                        if (amdgpu_asic_read_register(adev, se_num, sh_num,
                                                      info->read_mmr_reg.dword_offset + i,
                                                      &regs[i])) {
                                DRM_DEBUG_KMS("unallowed offset %#x\n",
                                              info->read_mmr_reg.dword_offset + i);
-                               kfree(regs);
-                               amdgpu_gfx_off_ctrl(adev, true);
                                ret = -EFAULT;
-                               goto out;
+                               break;
                        }
                }
                amdgpu_gfx_off_ctrl(adev, true);
-               n = copy_to_user(out, regs, min(size, alloc_size));
-               kfree(regs);
-               ret = (n ? -EFAULT : 0);
-out:
                up_read(&adev->reset_domain->sem);
+
+               if (!ret) {
+                       ret = copy_to_user(out, regs, min(size, alloc_size))
+                               ? -EFAULT : 0;
+               }
+               kfree(regs);
                return ret;
        }
        case AMDGPU_INFO_DEV_INFO: {