drm/amdgpu: potential NULL dereference on error
Subsystems (3)
PATCH 1/1 drm/amdgpu: potential NULL dereference on error
debugfs_create_file() can return an error pointer if debugfs is disabled or it can return NULL on error.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2 -0
@@ -1980,6 +1980,8 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) adev, &amdgpu_debugfs_regs_fops);if (IS_ERR (ent))return PTR_ERR (ent); +if (!ent) +return -ENOMEM ;i_size_write (ent->d_inode, adev->rmmio_size); adev->debugfs_regs = ent;
Am 11.06.2015 10:49, schrieb Dan Carpenter:
Quoted 10 lines
debugfs_create_file()can return an error pointer if debugfs is disabled or it can return NULL on error.Signed-off-by Dan Carpenter <[email protected]>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) · line 1986
Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-0@@ -1980,6 +1980,8 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) adev, &amdgpu_debugfs_regs_fops);if (IS_ERR (ent))return PTR_ERR (ent); +if (!ent) +return -ENOMEM ;i_size_write (ent->d_inode, adev->rmmio_size); adev->debugfs_regs = ent;
would PTR_ERR_OR_ZERO() by an option ?
on the other hand,
why does debugfs_create_file() does not return -ENOMEN instead of NULL ?
re,
wh
Quoted 5 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-1--To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 25 lines · walter harms
Am 11.06.2015 10:49, schrieb Dan Carpenter:
Quoted 10 lines
debugfs_create_file()can return an error pointer if debugfs is disabled or it can return NULL on error.Signed-off-by Dan Carpenter <[email protected]>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) · line 1986
Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-0@@ -1980,6 +1980,8 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) adev, &amdgpu_debugfs_regs_fops);if (IS_ERR (ent))return PTR_ERR (ent); +if (!ent) +return -ENOMEM ;i_size_write (ent->d_inode, adev->rmmio_size); adev->debugfs_regs = ent;would
PTR_ERR_OR_ZERO()by an option ?
Quoted 3 lines
on the other hand,
why doesdebugfs_create_file()does not return-ENOMENinstead of NULL ?
Actually if debugfs is disabled then we should probably carry on. Let me change it to:
if (IS_ERR(ent)) return 0; if (!ent) return -ENOMEM;
regards,
dan carpenter
Am 11.06.2015 14:20, schrieb Dan Carpenter:
Quoted 30 lines · walter harms
Quoted 25 lines
Am 11.06.2015 10:49, schrieb Dan Carpenter:
Quoted 10 lines
debugfs_create_file()can return an error pointer if debugfs is disabled or it can return NULL on error.Signed-off-by Dan Carpenter <[email protected]>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +0
-0drivers/gpu/drm/amd/amdgpu/amdgpu_device.c static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) · line 1986
Quoted 7 lines
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +2
-0@@ -1980,6 +1980,8 @@ static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) adev, &amdgpu_debugfs_regs_fops);if (IS_ERR (ent))return PTR_ERR (ent); +if (!ent) +return -ENOMEM ;i_size_write (ent->d_inode, adev->rmmio_size); adev->debugfs_regs = ent;would
PTR_ERR_OR_ZERO()by an option ?Quoted 3 lines
on the other hand,
why doesdebugfs_create_file()does not return-ENOMENinstead of NULL ?
Quoted 9 lines
Actually if debugfs is disabled then we should probably carry on. Let me change it to:
if (IS_ERR(ent)) return 0; if (!ent) return -ENOMEM;
You still have to check 2 types of error return here.
I simply do not understand why ebugfs_create_file() does not return -ENOMEM (or returns NULL on any error).
re,
wh
Quoted 3 lines · walter harms
You still have to check 2 types of error return here.
I simply do not understand whyebugfs_create_file()does not return-ENOMEM(or returns NULL on any error).
To be honest, I don't know why debugfs_create_file() doesn't just return NULL when it is configured out. I think I have asked this before...
I think the answer is that it seemed like a good idea at the time. These days we would probably prefer to use:
if (enabled(CONFIG_DEBUGFS)) {to test if it's there or not. Maybe that's still the right thing to check here.
But debugfs error handling is designed so that under normal situations you don't have to check for errors. It turns out that everyone still does because they are used to checking for errors.
The only reason we have to check here is because we do:
i_size_write(ent->d_inode, adev->rmmio_size);
^^^^^^^^^^^^Dereference.
regards,
dan carpenter