[PATCH 1/1] drm/amdgpu: ignore -EPERM error from debugfs

[Lazar, Lijo]

On 10/5/2021 7:19 PM, Das, Nirmoy wrote:
> 
> On 10/5/2021 3:22 PM, Christian König wrote:
>>
>>
>> Am 05.10.21 um 15:11 schrieb Nirmoy Das:
>>> Debugfs core APIs will throw -EPERM when user disables debugfs
>>> using CONFIG_DEBUG_FS_ALLOW_NONE or with kernel param. We shouldn't
>>> see that as an error. Also validate drm root dentry before creating
>>> amdgpu debugfs files.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
>>> ---
>>>   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 6611b3c7c149..d786072e918b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
>>> @@ -1617,6 +1617,16 @@ int amdgpu_debugfs_init(struct amdgpu_device 
>>> *adev)
>>>       struct dentry *ent;
>>>       int r, i;
>>>   +    if (IS_ERR(root)) {
>>> +        /* When debugfs is disabled we get -EPERM which is not an
>>> +         * error as this is user controllable.
>>> +         */
>>
>> Well setting primary->debugfs_root to an error code is probably not a 
>> good idea to begin with.
>>
>> When debugfs is disabled that should most likely be NULL.
> 
> 
> If we set primary->debugfs_root to  NULL then we need to add bunch of 
> NULL checks everywhere before creating any debugfs files
> 
> because debugfs_create_{file|dir}() with NULL root is still valid.  I am 
> assuming a hypothetical case when debugfs_root dir creation fails  even 
> with debugfs enabled
> 
> but further calls are successful.  This wont be a problem if we 
> propagate the error code.
> 
> 

drm does a blind assignment.

	minor->debugfs_root = debugfs_create_dir(name, root);

As per the latest documentation, the API doesn't return NULL
	If an error occurs, ERR_PTR(-ERROR) will be returned.

In short, root won't be NULL once the debugfs API is called.

Thanks,
Lijo

> Regards,
> 
> Nirmoy
> 
>>
>> Regards,
>> Christian.
>>
>>> +        if (PTR_ERR(root) == -EPERM)
>>> +            return 0;
>>> +
>>> +        return PTR_ERR(ent);
>>> +    }
>>> +
>>>       ent = debugfs_create_file("amdgpu_preempt_ib", 0600, root, adev,
>>>                     &fops_ib_preempt);
>>>       if (IS_ERR(ent)) {
>>