Am 14.06.2018 um 16:45 schrieb Dan Carpenter:
> The error handling is wrong and "ent" could be NULL we when dereference
> it to get "ent->d_inode".
>
> The thing is that normally debugfs_create_file() is not supposed to
> require (or have) any error handling. That function does return error
> pointers if debugfs is turned off but we know it's enable here. When
> it's enabled, then it returns NULL on error.
>
> So what I did was I stripped out all the error handling except around
> the i_size_write(). I could have just used a NULL check instead of an
> IS_ERR_OR_NULL() but I figured this was more clear because that way you
> don't have to look at the surrounding code to see whether debugfs is
> enabled or not.
Stumbled over this while going through old mails. Patch is Reviewed and
pushed.
Thanks,
Christian.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index f5fb93795a69..dd9a4fb9ce39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -826,21 +826,13 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
> {
> struct drm_minor *minor = adev->ddev->primary;
> struct dentry *ent, *root = minor->debugfs_root;
> - unsigned i, j;
> + unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(debugfs_regs); i++) {
> ent = debugfs_create_file(debugfs_regs_names[i],
> S_IFREG | S_IRUGO, root,
> adev, debugfs_regs[i]);
> - if (IS_ERR(ent)) {
> - for (j = 0; j < i; j++) {
> - debugfs_remove(adev->debugfs_regs[i]);
> - adev->debugfs_regs[i] = NULL;
> - }
> - return PTR_ERR(ent);
> - }
> -
> - if (!i)
> + if (!i && !IS_ERR_OR_NULL(ent))
> i_size_write(ent->d_inode, adev->rmmio_size);
> adev->debugfs_regs[i] = ent;
> }
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
