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)

authored by

Christian König and committed by

Alex Deucher 1 month ago 0ef196a2 ccf8932e

+24 -33

1 changed file

expand all

drivers

gpu

drm

amd

amdgpu

amdgpu_kms.c

+24 -33

drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

··· 873 873 ? -EFAULT : 0; 874 874 } 875 875 case AMDGPU_INFO_READ_MMR_REG: { 876 - int ret = 0; 877 - unsigned int n, alloc_size; 878 - uint32_t *regs; 879 876 unsigned int se_num = (info->read_mmr_reg.instance >> 880 877 AMDGPU_INFO_MMR_SE_INDEX_SHIFT) & 881 878 AMDGPU_INFO_MMR_SE_INDEX_MASK; 882 879 unsigned int sh_num = (info->read_mmr_reg.instance >> 883 880 AMDGPU_INFO_MMR_SH_INDEX_SHIFT) & 884 881 AMDGPU_INFO_MMR_SH_INDEX_MASK; 885 - 886 - if (!down_read_trylock(&adev->reset_domain->sem)) 887 - return -ENOENT; 882 + unsigned int alloc_size; 883 + uint32_t *regs; 884 + int ret; 888 885 889 886 /* set full masks if the userspace set all bits 890 887 * in the bitfields 891 888 */ 892 - if (se_num == AMDGPU_INFO_MMR_SE_INDEX_MASK) { 889 + if (se_num == AMDGPU_INFO_MMR_SE_INDEX_MASK) 893 890 se_num = 0xffffffff; 894 - } else if (se_num >= AMDGPU_GFX_MAX_SE) { 895 - ret = -EINVAL; 896 - goto out; 897 - } 891 + else if (se_num >= AMDGPU_GFX_MAX_SE) 892 + return -EINVAL; 898 893 899 - if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) { 894 + if (sh_num == AMDGPU_INFO_MMR_SH_INDEX_MASK) 900 895 sh_num = 0xffffffff; 901 - } else if (sh_num >= AMDGPU_GFX_MAX_SH_PER_SE) { 902 - ret = -EINVAL; 903 - goto out; 904 - } 896 + else if (sh_num >= AMDGPU_GFX_MAX_SH_PER_SE) 897 + return -EINVAL; 905 898 906 - if (info->read_mmr_reg.count > 128) { 907 - ret = -EINVAL; 908 - goto out; 909 - } 899 + if (info->read_mmr_reg.count > 128) 900 + return -EINVAL; 910 901 911 - regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), GFP_KERNEL); 912 - if (!regs) { 913 - ret = -ENOMEM; 914 - goto out; 915 - } 902 + regs = kmalloc_array(info->read_mmr_reg.count, sizeof(*regs), 903 + GFP_KERNEL); 904 + if (!regs) 905 + return -ENOMEM; 916 906 907 + down_read(&adev->reset_domain->sem); 917 908 alloc_size = info->read_mmr_reg.count * sizeof(*regs); 918 - 919 909 amdgpu_gfx_off_ctrl(adev, false); 910 + ret = 0; 920 911 for (i = 0; i < info->read_mmr_reg.count; i++) { 921 912 if (amdgpu_asic_read_register(adev, se_num, sh_num, 922 913 info->read_mmr_reg.dword_offset + i, 923 914 &regs[i])) { 924 915 DRM_DEBUG_KMS("unallowed offset %#x\n", 925 916 info->read_mmr_reg.dword_offset + i); 926 - kfree(regs); 927 - amdgpu_gfx_off_ctrl(adev, true); 928 917 ret = -EFAULT; 929 - goto out; 918 + break; 930 919 } 931 920 } 932 921 amdgpu_gfx_off_ctrl(adev, true); 933 - n = copy_to_user(out, regs, min(size, alloc_size)); 934 - kfree(regs); 935 - ret = (n ? -EFAULT : 0); 936 - out: 937 922 up_read(&adev->reset_domain->sem); 923 + 924 + if (!ret) { 925 + ret = copy_to_user(out, regs, min(size, alloc_size)) 926 + ? -EFAULT : 0; 927 + } 928 + kfree(regs); 938 929 return ret; 939 930 } 940 931 case AMDGPU_INFO_DEV_INFO: {

Configure Feed

Configure Feed