* [bug report] drm/ttm: fix re-init of global structures
@ 2020-02-04 12:57 Dan Carpenter
2020-02-04 14:03 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-02-04 12:57 UTC (permalink / raw)
To: christian.koenig; +Cc: dri-devel
Hello Christian König,
The patch bd4264112f93: "drm/ttm: fix re-init of global structures"
from Apr 16, 2019, leads to the following static checker warning:
drivers/gpu/drm/ttm/ttm_bo.c:1610 ttm_bo_global_release()
warn: passing freed memory 'glob'
drivers/gpu/drm/ttm/ttm_bo.c
1591 static void ttm_bo_global_kobj_release(struct kobject *kobj)
1592 {
1593 struct ttm_bo_global *glob =
1594 container_of(kobj, struct ttm_bo_global, kobj);
1595
1596 __free_page(glob->dummy_read_page);
1597 }
1598
1599 static void ttm_bo_global_release(void)
1600 {
1601 struct ttm_bo_global *glob = &ttm_bo_glob;
1602
1603 mutex_lock(&ttm_global_mutex);
1604 if (--ttm_bo_glob_use_count > 0)
1605 goto out;
1606
1607 kobject_del(&glob->kobj);
1608 kobject_put(&glob->kobj);
1609 ttm_mem_global_release(&ttm_mem_glob);
1610 memset(glob, 0, sizeof(*glob));
^^^^^^^^^^^^^^^^^^^^^^
Depending on the config kobject_release() might call ttm_bo_global_kobj_release()
a few seconds after this memset. Maybe put the memset into
ttm_bo_global_kobj_release()?
1611 out:
1612 mutex_unlock(&ttm_global_mutex);
1613 }
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] drm/ttm: fix re-init of global structures
2020-02-04 12:57 [bug report] drm/ttm: fix re-init of global structures Dan Carpenter
@ 2020-02-04 14:03 ` Christian König
2020-02-04 14:24 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Christian König @ 2020-02-04 14:03 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
Am 04.02.20 um 13:57 schrieb Dan Carpenter:
> Hello Christian König,
>
> The patch bd4264112f93: "drm/ttm: fix re-init of global structures"
> from Apr 16, 2019, leads to the following static checker warning:
>
> drivers/gpu/drm/ttm/ttm_bo.c:1610 ttm_bo_global_release()
> warn: passing freed memory 'glob'
>
> drivers/gpu/drm/ttm/ttm_bo.c
> 1591 static void ttm_bo_global_kobj_release(struct kobject *kobj)
> 1592 {
> 1593 struct ttm_bo_global *glob =
> 1594 container_of(kobj, struct ttm_bo_global, kobj);
> 1595
> 1596 __free_page(glob->dummy_read_page);
> 1597 }
> 1598
> 1599 static void ttm_bo_global_release(void)
> 1600 {
> 1601 struct ttm_bo_global *glob = &ttm_bo_glob;
> 1602
> 1603 mutex_lock(&ttm_global_mutex);
> 1604 if (--ttm_bo_glob_use_count > 0)
> 1605 goto out;
> 1606
> 1607 kobject_del(&glob->kobj);
> 1608 kobject_put(&glob->kobj);
> 1609 ttm_mem_global_release(&ttm_mem_glob);
> 1610 memset(glob, 0, sizeof(*glob));
> ^^^^^^^^^^^^^^^^^^^^^^
> Depending on the config kobject_release() might call ttm_bo_global_kobj_release()
> a few seconds after this memset. Maybe put the memset into
> ttm_bo_global_kobj_release()?
That's not possible. The object might be re-used directly after we drop
the ttm_global_mutex.
How can we wait for the ttm_mem_global_release() to have finished?
I mean in theory that function should actually be used from a
module_exit() callback, and we need to make 100% sure that the kobj is
gone or we are running in a bunch of trouble.
Christian.
>
> 1611 out:
> 1612 mutex_unlock(&ttm_global_mutex);
> 1613 }
>
>
> regards,
> dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] drm/ttm: fix re-init of global structures
2020-02-04 14:03 ` Christian König
@ 2020-02-04 14:24 ` Dan Carpenter
2020-02-04 14:27 ` Christian König
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2020-02-04 14:24 UTC (permalink / raw)
To: Christian König; +Cc: dri-devel
On Tue, Feb 04, 2020 at 03:03:43PM +0100, Christian König wrote:
> Am 04.02.20 um 13:57 schrieb Dan Carpenter:
> > Hello Christian König,
> >
> > The patch bd4264112f93: "drm/ttm: fix re-init of global structures"
> > from Apr 16, 2019, leads to the following static checker warning:
> >
> > drivers/gpu/drm/ttm/ttm_bo.c:1610 ttm_bo_global_release()
> > warn: passing freed memory 'glob'
> >
> > drivers/gpu/drm/ttm/ttm_bo.c
> > 1591 static void ttm_bo_global_kobj_release(struct kobject *kobj)
> > 1592 {
> > 1593 struct ttm_bo_global *glob =
> > 1594 container_of(kobj, struct ttm_bo_global, kobj);
> > 1595
> > 1596 __free_page(glob->dummy_read_page);
> > 1597 }
> > 1598
> > 1599 static void ttm_bo_global_release(void)
> > 1600 {
> > 1601 struct ttm_bo_global *glob = &ttm_bo_glob;
> > 1602
> > 1603 mutex_lock(&ttm_global_mutex);
> > 1604 if (--ttm_bo_glob_use_count > 0)
> > 1605 goto out;
> > 1606
> > 1607 kobject_del(&glob->kobj);
> > 1608 kobject_put(&glob->kobj);
> > 1609 ttm_mem_global_release(&ttm_mem_glob);
> > 1610 memset(glob, 0, sizeof(*glob));
> > ^^^^^^^^^^^^^^^^^^^^^^
> > Depending on the config kobject_release() might call ttm_bo_global_kobj_release()
> > a few seconds after this memset. Maybe put the memset into
> > ttm_bo_global_kobj_release()?
>
> That's not possible. The object might be re-used directly after we drop the
> ttm_global_mutex.
>
Hm... That sucks. If we reallocate glob->dummy_read_page before the
ttm_bo_global_kobj_release() gets called then we're toasted.
> How can we wait for the ttm_mem_global_release() to have finished?
>
A bunch of these release functions use a completion. But you probably
don't want a four second delay before we can re-use the struct.
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] drm/ttm: fix re-init of global structures
2020-02-04 14:24 ` Dan Carpenter
@ 2020-02-04 14:27 ` Christian König
0 siblings, 0 replies; 4+ messages in thread
From: Christian König @ 2020-02-04 14:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dri-devel
Am 04.02.20 um 15:24 schrieb Dan Carpenter:
> On Tue, Feb 04, 2020 at 03:03:43PM +0100, Christian König wrote:
>> Am 04.02.20 um 13:57 schrieb Dan Carpenter:
>>> Hello Christian König,
>>>
>>> The patch bd4264112f93: "drm/ttm: fix re-init of global structures"
>>> from Apr 16, 2019, leads to the following static checker warning:
>>>
>>> drivers/gpu/drm/ttm/ttm_bo.c:1610 ttm_bo_global_release()
>>> warn: passing freed memory 'glob'
>>>
>>> drivers/gpu/drm/ttm/ttm_bo.c
>>> 1591 static void ttm_bo_global_kobj_release(struct kobject *kobj)
>>> 1592 {
>>> 1593 struct ttm_bo_global *glob =
>>> 1594 container_of(kobj, struct ttm_bo_global, kobj);
>>> 1595
>>> 1596 __free_page(glob->dummy_read_page);
>>> 1597 }
>>> 1598
>>> 1599 static void ttm_bo_global_release(void)
>>> 1600 {
>>> 1601 struct ttm_bo_global *glob = &ttm_bo_glob;
>>> 1602
>>> 1603 mutex_lock(&ttm_global_mutex);
>>> 1604 if (--ttm_bo_glob_use_count > 0)
>>> 1605 goto out;
>>> 1606
>>> 1607 kobject_del(&glob->kobj);
>>> 1608 kobject_put(&glob->kobj);
>>> 1609 ttm_mem_global_release(&ttm_mem_glob);
>>> 1610 memset(glob, 0, sizeof(*glob));
>>> ^^^^^^^^^^^^^^^^^^^^^^
>>> Depending on the config kobject_release() might call ttm_bo_global_kobj_release()
>>> a few seconds after this memset. Maybe put the memset into
>>> ttm_bo_global_kobj_release()?
>> That's not possible. The object might be re-used directly after we drop the
>> ttm_global_mutex.
>>
> Hm... That sucks. If we reallocate glob->dummy_read_page before the
> ttm_bo_global_kobj_release() gets called then we're toasted.
>
>> How can we wait for the ttm_mem_global_release() to have finished?
>>
> A bunch of these release functions use a completion. But you probably
> don't want a four second delay before we can re-use the struct.
Actually that should be fine.
I mean the function is usually called on module unload, if that really
waits for 4 seconds until it calls ttm_bo_global_kobj_release() then
that would most likely result in a crash anyway because the code segment
is already unloaded.
Regards,
Christian.
>
> regards,
> dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-04 14:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-04 12:57 [bug report] drm/ttm: fix re-init of global structures Dan Carpenter
2020-02-04 14:03 ` Christian König
2020-02-04 14:24 ` Dan Carpenter
2020-02-04 14:27 ` 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.