All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] drm/ttm: Fix dummy res NULL ptr deref bug
@ 2022-08-11  9:55 Dan Carpenter
  2022-08-11 11:06 ` Arunpravin Paneer Selvam
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-08-11  9:55 UTC (permalink / raw)
  To: Arunpravin.PaneerSelvam; +Cc: dri-devel

Hello Arunpravin Paneer Selvam,

This is a semi-automatic email about new static checker warnings.

The patch cf4b7387c0a8: "drm/ttm: Fix dummy res NULL ptr deref bug"
from Aug 9, 2022, leads to the following Smatch complaint:

    drivers/gpu/drm/ttm/ttm_bo.c:915 ttm_bo_validate()
    warn: variable dereferenced before check 'bo->resource' (see line 907)

drivers/gpu/drm/ttm/ttm_bo.c
   906		 */
   907		if (!ttm_resource_compat(bo->resource, placement)) {
                                         ^^^^^^^^^^^^
Unchecked dereference here inside the function.

   908			ret = ttm_bo_move_buffer(bo, placement, ctx);
   909			if (ret)
   910				return ret;
   911		}
   912		/*
   913		 * We might need to add a TTM.
   914		 */
   915		if (!bo->resource || bo->resource->mem_type == TTM_PL_SYSTEM) {
                     ^^^^^^^^^^^^
Checked too late.

This NULL check was added deliberately based on a report from the kbuild
bot, but it's not clear why bo->resource is NULL at this point.  Was the
patch tested?  There is a stable@vger.kernel.org but there is no Fixes
tag.

   916			ret = ttm_tt_create(bo, true);
   917			if (ret)

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] drm/ttm: Fix dummy res NULL ptr deref bug
  2022-08-11  9:55 [bug report] drm/ttm: Fix dummy res NULL ptr deref bug Dan Carpenter
@ 2022-08-11 11:06 ` Arunpravin Paneer Selvam
  2022-08-11 11:56   ` Dan Carpenter
  0 siblings, 1 reply; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-08-11 11:06 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: dri-devel

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

Hi Dan,

drm-misc-fixes doesn't have the updated ttm_bo.c file, we have the 
updated ttm_bo.c version in
drm-misc-next branch. Please find below for the line number 907.

On 8/11/2022 3:25 PM, Dan Carpenter wrote:
> Hello Arunpravin Paneer Selvam,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch cf4b7387c0a8: "drm/ttm: Fix dummy res NULL ptr deref bug"
> from Aug 9, 2022, leads to the following Smatch complaint:
>
>      drivers/gpu/drm/ttm/ttm_bo.c:915 ttm_bo_validate()
>      warn: variable dereferenced before check 'bo->resource' (see line 907)
>
> drivers/gpu/drm/ttm/ttm_bo.c
>     906		 */
>     907		if (!ttm_resource_compat(bo->resource, placement)) {
>                                           ^^^^^^^^^^^^
> Unchecked dereference here inside the function.

|if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { 
we have this version in drm-misc-next Regards, Arun |

>
>     908			ret = ttm_bo_move_buffer(bo, placement, ctx);
>     909			if (ret)
>     910				return ret;
>     911		}
>     912		/*
>     913		 * We might need to add a TTM.
>     914		 */
>     915		if (!bo->resource || bo->resource->mem_type == TTM_PL_SYSTEM) {
>                       ^^^^^^^^^^^^
> Checked too late.
>
> This NULL check was added deliberately based on a report from the kbuild
> bot, but it's not clear why bo->resource is NULL at this point.  Was the
> patch tested?  There is astable@vger.kernel.org  but there is no Fixes
> tag.
>
>     916			ret = ttm_tt_create(bo, true);
>     917			if (ret)
>
> regards,
> dan carpenter

[-- Attachment #2: Type: text/html, Size: 2597 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] drm/ttm: Fix dummy res NULL ptr deref bug
  2022-08-11 11:06 ` Arunpravin Paneer Selvam
@ 2022-08-11 11:56   ` Dan Carpenter
  2022-08-14  6:00     ` Arunpravin Paneer Selvam
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-08-11 11:56 UTC (permalink / raw)
  To: Arunpravin Paneer Selvam; +Cc: dri-devel

On Thu, Aug 11, 2022 at 04:36:33PM +0530, Arunpravin Paneer Selvam wrote:
> Hi Dan,
> 
> drm-misc-fixes doesn't have the updated ttm_bo.c file, we have the updated
> ttm_bo.c version in
> drm-misc-next branch. Please find below for the line number 907.
> 
> On 8/11/2022 3:25 PM, Dan Carpenter wrote:
> > Hello Arunpravin Paneer Selvam,
> > 
> > This is a semi-automatic email about new static checker warnings.
> > 
> > The patch cf4b7387c0a8: "drm/ttm: Fix dummy res NULL ptr deref bug"
> > from Aug 9, 2022, leads to the following Smatch complaint:
> > 
> >      drivers/gpu/drm/ttm/ttm_bo.c:915 ttm_bo_validate()
> >      warn: variable dereferenced before check 'bo->resource' (see line 907)
> > 
> > drivers/gpu/drm/ttm/ttm_bo.c
> >     906		 */
> >     907		if (!ttm_resource_compat(bo->resource, placement)) {
> >                                           ^^^^^^^^^^^^
> > Unchecked dereference here inside the function.
> 
> |if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { we
> have this version in drm-misc-next Regards, Arun |
> 

Huh...  That's very interesting.  It appears there was a bug in
drm-misc-next, we applied the fix to the wrong tree, and now both trees
are wrong.  The drm-misc-next tree still has the bug and the other tree
has a static checker warning about nonsensical NULL checks.

Eventually drm-misc-next will get merged and everything will work.  Is
it too late to remove the bogus "CC: stable@vger.kernel.org"?

This could have been avoided if the NULL dereference fix had a Fixes tag.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] drm/ttm: Fix dummy res NULL ptr deref bug
  2022-08-11 11:56   ` Dan Carpenter
@ 2022-08-14  6:00     ` Arunpravin Paneer Selvam
  2022-08-14 17:50       ` Christian König
  0 siblings, 1 reply; 5+ messages in thread
From: Arunpravin Paneer Selvam @ 2022-08-14  6:00 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Christian König, dri-devel

Hi Dan,

On 8/11/2022 5:26 PM, Dan Carpenter wrote:
> On Thu, Aug 11, 2022 at 04:36:33PM +0530, Arunpravin Paneer Selvam wrote:
>> Hi Dan,
>>
>> drm-misc-fixes doesn't have the updated ttm_bo.c file, we have the updated
>> ttm_bo.c version in
>> drm-misc-next branch. Please find below for the line number 907.
>>
>> On 8/11/2022 3:25 PM, Dan Carpenter wrote:
>>> Hello Arunpravin Paneer Selvam,
>>>
>>> This is a semi-automatic email about new static checker warnings.
>>>
>>> The patch cf4b7387c0a8: "drm/ttm: Fix dummy res NULL ptr deref bug"
>>> from Aug 9, 2022, leads to the following Smatch complaint:
>>>
>>>       drivers/gpu/drm/ttm/ttm_bo.c:915 ttm_bo_validate()
>>>       warn: variable dereferenced before check 'bo->resource' (see line 907)
>>>
>>> drivers/gpu/drm/ttm/ttm_bo.c
>>>      906		 */
>>>      907		if (!ttm_resource_compat(bo->resource, placement)) {
>>>                                            ^^^^^^^^^^^^
>>> Unchecked dereference here inside the function.
>> |if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) { we
>> have this version in drm-misc-next Regards, Arun |
>>
> Huh...  That's very interesting.  It appears there was a bug in
> drm-misc-next, we applied the fix to the wrong tree, and now both trees
> are wrong.  The drm-misc-next tree still has the bug and the other tree
> has a static checker warning about nonsensical NULL checks.
>
> Eventually drm-misc-next will get merged and everything will work.  Is
> it too late to remove the bogus "CC: stable@vger.kernel.org"?
I will look into this problem.
> This could have been avoided if the NULL dereference fix had a Fixes tag.
I should have added the below tag
Fixes: 347987a2cf0d ("drm/ttm: rename and cleanup ttm_bo_init")

I will check on this.

Thanks,
Arun
>
> regards,
> dan carpenter
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] drm/ttm: Fix dummy res NULL ptr deref bug
  2022-08-14  6:00     ` Arunpravin Paneer Selvam
@ 2022-08-14 17:50       ` Christian König
  0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2022-08-14 17:50 UTC (permalink / raw)
  To: Arunpravin Paneer Selvam, Dan Carpenter; +Cc: dri-devel

Am 14.08.22 um 08:00 schrieb Arunpravin Paneer Selvam:
> Hi Dan,
>
> On 8/11/2022 5:26 PM, Dan Carpenter wrote:
>> On Thu, Aug 11, 2022 at 04:36:33PM +0530, Arunpravin Paneer Selvam 
>> wrote:
>>> Hi Dan,
>>>
>>> drm-misc-fixes doesn't have the updated ttm_bo.c file, we have the 
>>> updated
>>> ttm_bo.c version in
>>> drm-misc-next branch. Please find below for the line number 907.
>>>
>>> On 8/11/2022 3:25 PM, Dan Carpenter wrote:
>>>> Hello Arunpravin Paneer Selvam,
>>>>
>>>> This is a semi-automatic email about new static checker warnings.
>>>>
>>>> The patch cf4b7387c0a8: "drm/ttm: Fix dummy res NULL ptr deref bug"
>>>> from Aug 9, 2022, leads to the following Smatch complaint:
>>>>
>>>>       drivers/gpu/drm/ttm/ttm_bo.c:915 ttm_bo_validate()
>>>>       warn: variable dereferenced before check 'bo->resource' (see 
>>>> line 907)
>>>>
>>>> drivers/gpu/drm/ttm/ttm_bo.c
>>>>      906         */
>>>>      907        if (!ttm_resource_compat(bo->resource, placement)) {
>>>>                                            ^^^^^^^^^^^^
>>>> Unchecked dereference here inside the function.
>>> |if (!bo->resource || !ttm_resource_compat(bo->resource, placement)) 
>>> { we
>>> have this version in drm-misc-next Regards, Arun |
>>>
>> Huh...  That's very interesting.  It appears there was a bug in
>> drm-misc-next, we applied the fix to the wrong tree, and now both trees
>> are wrong.  The drm-misc-next tree still has the bug and the other tree
>> has a static checker warning about nonsensical NULL checks.
>>
>> Eventually drm-misc-next will get merged and everything will work.  Is
>> it too late to remove the bogus "CC: stable@vger.kernel.org"?
> I will look into this problem.

Mhm, if I'm not completely mistaken the "CC: stable@vger.kernel.org" is 
actually correct, we just need to limit to which version it applies.

>> This could have been avoided if the NULL dereference fix had a Fixes 
>> tag.
> I should have added the below tag
> Fixes: 347987a2cf0d ("drm/ttm: rename and cleanup ttm_bo_init")

WAIT! That's not the correct one. This patch just made the problem more 
obvious.

The real one is bfa3357ef9ab drm/ttm: allocate resource object instead 
of embedding it v2

Regards,
Christian.

>
> I will check on this.
>
> Thanks,
> Arun
>>
>> regards,
>> dan carpenter
>>
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-08-14 17:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-11  9:55 [bug report] drm/ttm: Fix dummy res NULL ptr deref bug Dan Carpenter
2022-08-11 11:06 ` Arunpravin Paneer Selvam
2022-08-11 11:56   ` Dan Carpenter
2022-08-14  6:00     ` Arunpravin Paneer Selvam
2022-08-14 17:50       ` Christian König

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.