All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11  8:49 ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11  8:49 UTC (permalink / raw)
  To: David Airlie
  Cc: Alex Deucher, Christian König, Jammy Zhou, yanyang1,
	Marek Olšák, dri-devel, kernel-janitors

debugfs_create_file() can return an error pointer if debugfs is disabled
or it can return NULL on error.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 36be03c..adba2a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -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;
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11  8:49 ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11  8:49 UTC (permalink / raw)
  To: David Airlie
  Cc: Alex Deucher, Christian König, Jammy Zhou, yanyang1,
	Marek Olšák, dri-devel, kernel-janitors

debugfs_create_file() can return an error pointer if debugfs is disabled
or it can return NULL on error.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 36be03c..adba2a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -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;
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
  2015-06-11  8:49 ` Dan Carpenter
@ 2015-06-11 12:03   ` walter harms
  -1 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2015-06-11 12:03 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors



Am 11.06.2015 10:49, schrieb Dan Carpenter:
> debugfs_create_file() can return an error pointer if debugfs is disabled
> or it can return NULL on error.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 36be03c..adba2a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -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


> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11 12:03   ` walter harms
  0 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2015-06-11 12:03 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors



Am 11.06.2015 10:49, schrieb Dan Carpenter:
> debugfs_create_file() can return an error pointer if debugfs is disabled
> or it can return NULL on error.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 36be03c..adba2a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -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


> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
  2015-06-11 12:03   ` walter harms
@ 2015-06-11 12:20     ` Dan Carpenter
  -1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11 12:20 UTC (permalink / raw)
  To: walter harms
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors

On Thu, Jun 11, 2015 at 02:03:18PM +0200, walter harms wrote:
> 
> 
> Am 11.06.2015 10:49, schrieb Dan Carpenter:
> > debugfs_create_file() can return an error pointer if debugfs is disabled
> > or it can return NULL on error.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 36be03c..adba2a1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -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 ?
> 

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11 12:20     ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11 12:20 UTC (permalink / raw)
  To: walter harms
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors

On Thu, Jun 11, 2015 at 02:03:18PM +0200, walter harms wrote:
> 
> 
> Am 11.06.2015 10:49, schrieb Dan Carpenter:
> > debugfs_create_file() can return an error pointer if debugfs is disabled
> > or it can return NULL on error.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 36be03c..adba2a1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -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 ?
> 

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
  2015-06-11 12:20     ` Dan Carpenter
@ 2015-06-11 14:35       ` walter harms
  -1 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2015-06-11 14:35 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors



Am 11.06.2015 14:20, schrieb Dan Carpenter:
> On Thu, Jun 11, 2015 at 02:03:18PM +0200, walter harms wrote:
>>
>>
>> Am 11.06.2015 10:49, schrieb Dan Carpenter:
>>> debugfs_create_file() can return an error pointer if debugfs is disabled
>>> or it can return NULL on error.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 36be03c..adba2a1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -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 ?
>>
> 
> 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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11 14:35       ` walter harms
  0 siblings, 0 replies; 10+ messages in thread
From: walter harms @ 2015-06-11 14:35 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, Alex Deucher, Christian König, Jammy Zhou,
	yanyang1, Marek Olšák, dri-devel, kernel-janitors



Am 11.06.2015 14:20, schrieb Dan Carpenter:
> On Thu, Jun 11, 2015 at 02:03:18PM +0200, walter harms wrote:
>>
>>
>> Am 11.06.2015 10:49, schrieb Dan Carpenter:
>>> debugfs_create_file() can return an error pointer if debugfs is disabled
>>> or it can return NULL on error.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 36be03c..adba2a1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -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 ?
>>
> 
> 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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
  2015-06-11 14:35       ` walter harms
@ 2015-06-11 14:51         ` Dan Carpenter
  -1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11 14:51 UTC (permalink / raw)
  To: walter harms
  Cc: Marek Olšák, kernel-janitors, dri-devel, yanyang1,
	Alex Deucher, Christian König

On Thu, Jun 11, 2015 at 04:35:26PM +0200, walter harms wrote:
> 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).

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


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [patch] drm/amdgpu: potential NULL dereference on error
@ 2015-06-11 14:51         ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2015-06-11 14:51 UTC (permalink / raw)
  To: walter harms
  Cc: Marek Olšák, kernel-janitors, dri-devel, yanyang1,
	Alex Deucher, Christian König

On Thu, Jun 11, 2015 at 04:35:26PM +0200, walter harms wrote:
> 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).

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-06-11 14:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-11  8:49 [patch] drm/amdgpu: potential NULL dereference on error Dan Carpenter
2015-06-11  8:49 ` Dan Carpenter
2015-06-11 12:03 ` walter harms
2015-06-11 12:03   ` walter harms
2015-06-11 12:20   ` Dan Carpenter
2015-06-11 12:20     ` Dan Carpenter
2015-06-11 14:35     ` walter harms
2015-06-11 14:35       ` walter harms
2015-06-11 14:51       ` Dan Carpenter
2015-06-11 14:51         ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.