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 <[email protected]> > > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
