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
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
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
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@lists.freedesktop.org
-
Christian König -
Dan Carpenter