* [PATCH 1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug
2022-09-19 1:09 [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Adrián Larumbe
@ 2022-09-19 1:09 ` Adrián Larumbe
2022-09-19 12:57 ` Neil Armstrong
2022-09-19 1:09 ` [PATCH 2/3] drm/meson: explicitly remove aggregate driver at module unload time Adrián Larumbe
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Adrián Larumbe @ 2022-09-19 1:09 UTC (permalink / raw)
To: narmstrong, khilman, linux-amlogic, dri-devel; +Cc: adrian.larumbe
Unloading the driver triggers the following KASAN warning:
[ +0.006275] =============================================================
[ +0.000029] BUG: KASAN: use-after-free in __list_del_entry_valid+0xe0/0x1a0
[ +0.000026] Read of size 8 at addr ffff000020c395e0 by task rmmod/2695
[ +0.000019] CPU: 5 PID: 2695 Comm: rmmod Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
[ +0.000013] Hardware name: Hardkernel ODROID-N2Plus (DT)
[ +0.000008] Call trace:
[ +0.000007] dump_backtrace+0x1ec/0x280
[ +0.000013] show_stack+0x24/0x80
[ +0.000008] dump_stack_lvl+0x98/0xd4
[ +0.000011] print_address_description.constprop.0+0x80/0x520
[ +0.000011] print_report+0x128/0x260
[ +0.000007] kasan_report+0xb8/0xfc
[ +0.000008] __asan_report_load8_noabort+0x3c/0x50
[ +0.000010] __list_del_entry_valid+0xe0/0x1a0
[ +0.000009] drm_atomic_private_obj_fini+0x30/0x200 [drm]
[ +0.000172] drm_bridge_detach+0x94/0x260 [drm]
[ +0.000145] drm_encoder_cleanup+0xa4/0x290 [drm]
[ +0.000144] drm_mode_config_cleanup+0x118/0x740 [drm]
[ +0.000143] drm_mode_config_init_release+0x1c/0x2c [drm]
[ +0.000144] drm_managed_release+0x170/0x414 [drm]
[ +0.000142] drm_dev_put.part.0+0xc0/0x124 [drm]
[ +0.000143] drm_dev_put+0x20/0x30 [drm]
[ +0.000142] meson_drv_unbind+0x1d8/0x2ac [meson_drm]
[ +0.000028] take_down_aggregate_device+0xb0/0x160
[ +0.000016] component_del+0x18c/0x360
[ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[ +0.000015] platform_remove+0x64/0xb0
[ +0.000009] device_remove+0xb8/0x154
[ +0.000009] device_release_driver_internal+0x398/0x5b0
[ +0.000009] driver_detach+0xac/0x1b0
[ +0.000009] bus_remove_driver+0x158/0x29c
[ +0.000009] driver_unregister+0x70/0xb0
[ +0.000008] platform_driver_unregister+0x20/0x2c
[ +0.000008] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
[ +0.000012] __do_sys_delete_module+0x288/0x400
[ +0.000011] __arm64_sys_delete_module+0x5c/0x80
[ +0.000009] invoke_syscall+0x74/0x260
[ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000009] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000012] el0t_64_sync_handler+0x11c/0x150
[ +0.000008] el0t_64_sync+0x18c/0x190
[ +0.000018] Allocated by task 0:
[ +0.000007] (stack is not available)
[ +0.000011] Freed by task 2695:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000011] kasan_set_track+0x2c/0x40
[ +0.000008] kasan_set_free_info+0x28/0x50
[ +0.000009] ____kasan_slab_free+0x128/0x1d4
[ +0.000008] __kasan_slab_free+0x18/0x24
[ +0.000007] slab_free_freelist_hook+0x108/0x230
[ +0.000011] kfree+0x110/0x35c
[ +0.000008] release_nodes+0xf0/0x16c
[ +0.000009] devres_release_group+0x180/0x270
[ +0.000008] component_unbind+0x128/0x1e0
[ +0.000010] component_unbind_all+0x1b8/0x264
[ +0.000009] meson_drv_unbind+0x1a0/0x2ac [meson_drm]
[ +0.000025] take_down_aggregate_device+0xb0/0x160
[ +0.000009] component_del+0x18c/0x360
[ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[ +0.000012] platform_remove+0x64/0xb0
[ +0.000008] device_remove+0xb8/0x154
[ +0.000009] device_release_driver_internal+0x398/0x5b0
[ +0.000009] driver_detach+0xac/0x1b0
[ +0.000009] bus_remove_driver+0x158/0x29c
[ +0.000008] driver_unregister+0x70/0xb0
[ +0.000008] platform_driver_unregister+0x20/0x2c
[ +0.000008] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
[ +0.000011] __do_sys_delete_module+0x288/0x400
[ +0.000010] __arm64_sys_delete_module+0x5c/0x80
[ +0.000008] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000009] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000014] The buggy address belongs to the object at ffff000020c39000
which belongs to the cache kmalloc-4k of size 4096
[ +0.000008] The buggy address is located 1504 bytes inside of
4096-byte region [ffff000020c39000, ffff000020c3a000)
[ +0.000016] The buggy address belongs to the physical page:
[ +0.000009] page:fffffc0000830e00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x20c38
[ +0.000013] head:fffffc0000830e00 order:3 compound_mapcount:0 compound_pincount:0
[ +0.000008] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
[ +0.000019] raw: 0ffff00000010200 fffffc0000fd4808 fffffc0000126208 ffff000000002e80
[ +0.000009] raw: 0000000000000000 0000000000020002 00000001ffffffff 0000000000000000
[ +0.000008] page dumped because: kasan: bad access detected
[ +0.000011] Memory state around the buggy address:
[ +0.000008] ffff000020c39480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ffff000020c39500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] >ffff000020c39580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ^
[ +0.000007] ffff000020c39600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ffff000020c39680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000006] ==================================================================
The reason this is happening is unloading meson-dw-hdmi will cause the
component API to take down the aggregate device, which in turn will cause
all devres-managed memory to be freed, including the struct dw_hdmi
allocated in dw_hdmi_probe. This struct embeds a struct drm_bridge that is
added at the end of the function, and which is later on picked up in
meson_encoder_hdmi_init.
However, when attaching the bridge to the encoder created in
meson_encoder_hdmi_init, it's linked to the encoder's bridge chain, from
where it never leaves, even after devres_release_group is called when the
driver's components are unbound and the embedding structure freed.
Then, when calling drm_dev_put in the aggregate driver's unbind function,
drm_bridge_detach is called for every single bridge linked to the encoder,
including the one whose memory had already been deallocated.
Fix by calling component_unbind_all after drm_dev_put.
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
drivers/gpu/drm/meson/meson_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index ef386d7b9450..8da454a17b77 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -385,9 +385,9 @@ static void meson_drv_unbind(struct device *dev)
drm_dev_unregister(drm);
drm_kms_helper_poll_fini(drm);
drm_atomic_helper_shutdown(drm);
- component_unbind_all(dev, drm);
free_irq(priv->vsync_irq, drm);
drm_dev_put(drm);
+ component_unbind_all(dev, drm);
if (priv->afbcd.ops)
priv->afbcd.ops->exit(priv);
--
2.37.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug
2022-09-19 1:09 ` [PATCH 1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug Adrián Larumbe
@ 2022-09-19 12:57 ` Neil Armstrong
0 siblings, 0 replies; 12+ messages in thread
From: Neil Armstrong @ 2022-09-19 12:57 UTC (permalink / raw)
To: Adrián Larumbe, narmstrong, khilman, linux-amlogic, dri-devel
On 19/09/2022 03:09, Adrián Larumbe wrote:
> Unloading the driver triggers the following KASAN warning:
>
> [ +0.006275] =============================================================
> [ +0.000029] BUG: KASAN: use-after-free in __list_del_entry_valid+0xe0/0x1a0
> [ +0.000026] Read of size 8 at addr ffff000020c395e0 by task rmmod/2695
>
> [ +0.000019] CPU: 5 PID: 2695 Comm: rmmod Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
> [ +0.000013] Hardware name: Hardkernel ODROID-N2Plus (DT)
> [ +0.000008] Call trace:
> [ +0.000007] dump_backtrace+0x1ec/0x280
> [ +0.000013] show_stack+0x24/0x80
> [ +0.000008] dump_stack_lvl+0x98/0xd4
> [ +0.000011] print_address_description.constprop.0+0x80/0x520
> [ +0.000011] print_report+0x128/0x260
> [ +0.000007] kasan_report+0xb8/0xfc
> [ +0.000008] __asan_report_load8_noabort+0x3c/0x50
> [ +0.000010] __list_del_entry_valid+0xe0/0x1a0
> [ +0.000009] drm_atomic_private_obj_fini+0x30/0x200 [drm]
> [ +0.000172] drm_bridge_detach+0x94/0x260 [drm]
> [ +0.000145] drm_encoder_cleanup+0xa4/0x290 [drm]
> [ +0.000144] drm_mode_config_cleanup+0x118/0x740 [drm]
> [ +0.000143] drm_mode_config_init_release+0x1c/0x2c [drm]
> [ +0.000144] drm_managed_release+0x170/0x414 [drm]
> [ +0.000142] drm_dev_put.part.0+0xc0/0x124 [drm]
> [ +0.000143] drm_dev_put+0x20/0x30 [drm]
> [ +0.000142] meson_drv_unbind+0x1d8/0x2ac [meson_drm]
> [ +0.000028] take_down_aggregate_device+0xb0/0x160
> [ +0.000016] component_del+0x18c/0x360
> [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
> [ +0.000015] platform_remove+0x64/0xb0
> [ +0.000009] device_remove+0xb8/0x154
> [ +0.000009] device_release_driver_internal+0x398/0x5b0
> [ +0.000009] driver_detach+0xac/0x1b0
> [ +0.000009] bus_remove_driver+0x158/0x29c
> [ +0.000009] driver_unregister+0x70/0xb0
> [ +0.000008] platform_driver_unregister+0x20/0x2c
> [ +0.000008] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
> [ +0.000012] __do_sys_delete_module+0x288/0x400
> [ +0.000011] __arm64_sys_delete_module+0x5c/0x80
> [ +0.000009] invoke_syscall+0x74/0x260
> [ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000009] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000012] el0t_64_sync_handler+0x11c/0x150
> [ +0.000008] el0t_64_sync+0x18c/0x190
>
> [ +0.000018] Allocated by task 0:
> [ +0.000007] (stack is not available)
>
> [ +0.000011] Freed by task 2695:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000011] kasan_set_track+0x2c/0x40
> [ +0.000008] kasan_set_free_info+0x28/0x50
> [ +0.000009] ____kasan_slab_free+0x128/0x1d4
> [ +0.000008] __kasan_slab_free+0x18/0x24
> [ +0.000007] slab_free_freelist_hook+0x108/0x230
> [ +0.000011] kfree+0x110/0x35c
> [ +0.000008] release_nodes+0xf0/0x16c
> [ +0.000009] devres_release_group+0x180/0x270
> [ +0.000008] component_unbind+0x128/0x1e0
> [ +0.000010] component_unbind_all+0x1b8/0x264
> [ +0.000009] meson_drv_unbind+0x1a0/0x2ac [meson_drm]
> [ +0.000025] take_down_aggregate_device+0xb0/0x160
> [ +0.000009] component_del+0x18c/0x360
> [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
> [ +0.000012] platform_remove+0x64/0xb0
> [ +0.000008] device_remove+0xb8/0x154
> [ +0.000009] device_release_driver_internal+0x398/0x5b0
> [ +0.000009] driver_detach+0xac/0x1b0
> [ +0.000009] bus_remove_driver+0x158/0x29c
> [ +0.000008] driver_unregister+0x70/0xb0
> [ +0.000008] platform_driver_unregister+0x20/0x2c
> [ +0.000008] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
> [ +0.000011] __do_sys_delete_module+0x288/0x400
> [ +0.000010] __arm64_sys_delete_module+0x5c/0x80
> [ +0.000008] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000009] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000014] The buggy address belongs to the object at ffff000020c39000
> which belongs to the cache kmalloc-4k of size 4096
> [ +0.000008] The buggy address is located 1504 bytes inside of
> 4096-byte region [ffff000020c39000, ffff000020c3a000)
>
> [ +0.000016] The buggy address belongs to the physical page:
> [ +0.000009] page:fffffc0000830e00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x20c38
> [ +0.000013] head:fffffc0000830e00 order:3 compound_mapcount:0 compound_pincount:0
> [ +0.000008] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
> [ +0.000019] raw: 0ffff00000010200 fffffc0000fd4808 fffffc0000126208 ffff000000002e80
> [ +0.000009] raw: 0000000000000000 0000000000020002 00000001ffffffff 0000000000000000
> [ +0.000008] page dumped because: kasan: bad access detected
>
> [ +0.000011] Memory state around the buggy address:
> [ +0.000008] ffff000020c39480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ffff000020c39500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] >ffff000020c39580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ^
> [ +0.000007] ffff000020c39600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ffff000020c39680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000006] ==================================================================
>
> The reason this is happening is unloading meson-dw-hdmi will cause the
> component API to take down the aggregate device, which in turn will cause
> all devres-managed memory to be freed, including the struct dw_hdmi
> allocated in dw_hdmi_probe. This struct embeds a struct drm_bridge that is
> added at the end of the function, and which is later on picked up in
> meson_encoder_hdmi_init.
>
> However, when attaching the bridge to the encoder created in
> meson_encoder_hdmi_init, it's linked to the encoder's bridge chain, from
> where it never leaves, even after devres_release_group is called when the
> driver's components are unbound and the embedding structure freed.
>
> Then, when calling drm_dev_put in the aggregate driver's unbind function,
> drm_bridge_detach is called for every single bridge linked to the encoder,
> including the one whose memory had already been deallocated.
>
> Fix by calling component_unbind_all after drm_dev_put.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index ef386d7b9450..8da454a17b77 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -385,9 +385,9 @@ static void meson_drv_unbind(struct device *dev)
> drm_dev_unregister(drm);
> drm_kms_helper_poll_fini(drm);
> drm_atomic_helper_shutdown(drm);
> - component_unbind_all(dev, drm);
> free_irq(priv->vsync_irq, drm);
> drm_dev_put(drm);
> + component_unbind_all(dev, drm);
>
> if (priv->afbcd.ops)
> priv->afbcd.ops->exit(priv);
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/meson: explicitly remove aggregate driver at module unload time
2022-09-19 1:09 [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Adrián Larumbe
2022-09-19 1:09 ` [PATCH 1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug Adrián Larumbe
@ 2022-09-19 1:09 ` Adrián Larumbe
2022-09-19 12:57 ` Neil Armstrong
2022-09-19 1:09 ` [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time Adrián Larumbe
2022-09-23 9:48 ` (subset) [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Neil Armstrong
3 siblings, 1 reply; 12+ messages in thread
From: Adrián Larumbe @ 2022-09-19 1:09 UTC (permalink / raw)
To: narmstrong, khilman, linux-amlogic, dri-devel; +Cc: adrian.larumbe
Because component_master_del wasn't being called when unloading the
meson_drm module, the aggregate device would linger forever in the global
aggregate_devices list. That means when unloading and reloading the
meson_dw_hdmi module, component_add would call into
try_to_bring_up_aggregate_device and find the unbound meson_drm aggregate
device.
This would in turn dereference some of the aggregate_device's struct
entries which point to memory automatically freed by the devres API when
unbinding the aggregate device from meson_drv_unbind, and trigger an
use-after-free bug:
[ +0.000014] =============================================================
[ +0.000007] BUG: KASAN: use-after-free in find_components+0x468/0x500
[ +0.000017] Read of size 8 at addr ffff000006731688 by task modprobe/2536
[ +0.000018] CPU: 4 PID: 2536 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
[ +0.000010] Hardware name: Hardkernel ODROID-N2Plus (DT)
[ +0.000008] Call trace:
[ +0.000005] dump_backtrace+0x1ec/0x280
[ +0.000011] show_stack+0x24/0x80
[ +0.000007] dump_stack_lvl+0x98/0xd4
[ +0.000010] print_address_description.constprop.0+0x80/0x520
[ +0.000011] print_report+0x128/0x260
[ +0.000007] kasan_report+0xb8/0xfc
[ +0.000007] __asan_report_load8_noabort+0x3c/0x50
[ +0.000009] find_components+0x468/0x500
[ +0.000008] try_to_bring_up_aggregate_device+0x64/0x390
[ +0.000009] __component_add+0x1dc/0x49c
[ +0.000009] component_add+0x20/0x30
[ +0.000008] meson_dw_hdmi_probe+0x28/0x34 [meson_dw_hdmi]
[ +0.000013] platform_probe+0xd0/0x220
[ +0.000008] really_probe+0x3ac/0xa80
[ +0.000008] __driver_probe_device+0x1f8/0x400
[ +0.000008] driver_probe_device+0x68/0x1b0
[ +0.000008] __driver_attach+0x20c/0x480
[ +0.000009] bus_for_each_dev+0x114/0x1b0
[ +0.000007] driver_attach+0x48/0x64
[ +0.000009] bus_add_driver+0x390/0x564
[ +0.000007] driver_register+0x1a8/0x3e4
[ +0.000009] __platform_driver_register+0x6c/0x94
[ +0.000007] meson_dw_hdmi_platform_driver_init+0x30/0x1000 [meson_dw_hdmi]
[ +0.000014] do_one_initcall+0xc4/0x2b0
[ +0.000008] do_init_module+0x154/0x570
[ +0.000010] load_module+0x1a78/0x1ea4
[ +0.000008] __do_sys_init_module+0x184/0x1cc
[ +0.000008] __arm64_sys_init_module+0x78/0xb0
[ +0.000008] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000009] do_el0_svc+0x50/0x70
[ +0.000008] el0_svc+0x68/0x1a0
[ +0.000009] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000014] Allocated by task 902:
[ +0.000007] kasan_save_stack+0x2c/0x5c
[ +0.000009] __kasan_kmalloc+0x90/0xd0
[ +0.000007] __kmalloc_node+0x240/0x580
[ +0.000010] memcg_alloc_slab_cgroups+0xa4/0x1ac
[ +0.000010] memcg_slab_post_alloc_hook+0xbc/0x4c0
[ +0.000008] kmem_cache_alloc_node+0x1d0/0x490
[ +0.000009] __alloc_skb+0x1d4/0x310
[ +0.000010] alloc_skb_with_frags+0x8c/0x620
[ +0.000008] sock_alloc_send_pskb+0x5ac/0x6d0
[ +0.000010] unix_dgram_sendmsg+0x2e0/0x12f0
[ +0.000010] sock_sendmsg+0xcc/0x110
[ +0.000007] sock_write_iter+0x1d0/0x304
[ +0.000008] new_sync_write+0x364/0x460
[ +0.000007] vfs_write+0x420/0x5ac
[ +0.000008] ksys_write+0x19c/0x1f0
[ +0.000008] __arm64_sys_write+0x78/0xb0
[ +0.000007] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0x1a8/0x260
[ +0.000009] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000008] el0t_64_sync_handler+0x11c/0x150
[ +0.000008] el0t_64_sync+0x18c/0x190
[ +0.000013] Freed by task 2509:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000007] kasan_set_track+0x2c/0x40
[ +0.000008] kasan_set_free_info+0x28/0x50
[ +0.000008] ____kasan_slab_free+0x128/0x1d4
[ +0.000008] __kasan_slab_free+0x18/0x24
[ +0.000007] slab_free_freelist_hook+0x108/0x230
[ +0.000010] kfree+0x110/0x35c
[ +0.000008] release_nodes+0xf0/0x16c
[ +0.000008] devres_release_all+0xfc/0x180
[ +0.000008] device_unbind_cleanup+0x24/0x164
[ +0.000008] device_release_driver_internal+0x3e8/0x5b0
[ +0.000010] driver_detach+0xac/0x1b0
[ +0.000008] bus_remove_driver+0x158/0x29c
[ +0.000008] driver_unregister+0x70/0xb0
[ +0.000009] platform_driver_unregister+0x20/0x2c
[ +0.000007] 0xffff800003722d98
[ +0.000012] __do_sys_delete_module+0x288/0x400
[ +0.000009] __arm64_sys_delete_module+0x5c/0x80
[ +0.000008] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000008] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000013] Last potentially related work creation:
[ +0.000007] kasan_save_stack+0x2c/0x5c
[ +0.000007] __kasan_record_aux_stack+0xb8/0xf0
[ +0.000009] kasan_record_aux_stack_noalloc+0x14/0x20
[ +0.000008] insert_work+0x54/0x290
[ +0.000009] __queue_work+0x48c/0xd24
[ +0.000008] queue_work_on+0x90/0x11c
[ +0.000008] call_usermodehelper_exec+0x188/0x404
[ +0.000010] kobject_uevent_env+0x5a8/0x794
[ +0.000010] kobject_uevent+0x14/0x20
[ +0.000008] driver_register+0x230/0x3e4
[ +0.000009] __platform_driver_register+0x6c/0x94
[ +0.000007] gxbb_driver_init+0x28/0x34
[ +0.000010] do_one_initcall+0xc4/0x2b0
[ +0.000008] do_initcalls+0x20c/0x24c
[ +0.000010] kernel_init_freeable+0x22c/0x278
[ +0.000009] kernel_init+0x3c/0x170
[ +0.000008] ret_from_fork+0x10/0x20
[ +0.000013] The buggy address belongs to the object at ffff000006731600
which belongs to the cache kmalloc-256 of size 256
[ +0.000009] The buggy address is located 136 bytes inside of
256-byte region [ffff000006731600, ffff000006731700)
[ +0.000015] The buggy address belongs to the physical page:
[ +0.000008] page:fffffc000019cc00 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff000006730a00 pfn:0x6730
[ +0.000011] head:fffffc000019cc00 order:2 compound_mapcount:0 compound_pincount:0
[ +0.000008] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
[ +0.000016] raw: 0ffff00000010200 fffffc00000c3d08 fffffc0000ef2b08 ffff000000002680
[ +0.000009] raw: ffff000006730a00 0000000000150014 00000001ffffffff 0000000000000000
[ +0.000006] page dumped because: kasan: bad access detected
[ +0.000011] Memory state around the buggy address:
[ +0.000007] ffff000006731580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ +0.000007] ffff000006731600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] >ffff000006731680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ^
[ +0.000006] ffff000006731700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ +0.000007] ffff000006731780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ +0.000006] ==================================================================
Fix by adding 'remove' driver callback for meson-drm, and explicitly deleting the
aggregate device.
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
drivers/gpu/drm/meson/meson_drv.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 8da454a17b77..f3da1c214a7c 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -490,6 +490,13 @@ static int meson_drv_probe(struct platform_device *pdev)
return 0;
};
+static int meson_drv_remove(struct platform_device *pdev)
+{
+ component_master_del(&pdev->dev, &meson_drv_master_ops);
+
+ return 0;
+}
+
static struct meson_drm_match_data meson_drm_gxbb_data = {
.compat = VPU_COMPATIBLE_GXBB,
};
@@ -527,6 +534,7 @@ static const struct dev_pm_ops meson_drv_pm_ops = {
static struct platform_driver meson_drm_platform_driver = {
.probe = meson_drv_probe,
+ .remove = meson_drv_remove,
.shutdown = meson_drv_shutdown,
.driver = {
.name = "meson-drm",
--
2.37.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 2/3] drm/meson: explicitly remove aggregate driver at module unload time
2022-09-19 1:09 ` [PATCH 2/3] drm/meson: explicitly remove aggregate driver at module unload time Adrián Larumbe
@ 2022-09-19 12:57 ` Neil Armstrong
0 siblings, 0 replies; 12+ messages in thread
From: Neil Armstrong @ 2022-09-19 12:57 UTC (permalink / raw)
To: Adrián Larumbe, narmstrong, khilman, linux-amlogic, dri-devel
On 19/09/2022 03:09, Adrián Larumbe wrote:
> Because component_master_del wasn't being called when unloading the
> meson_drm module, the aggregate device would linger forever in the global
> aggregate_devices list. That means when unloading and reloading the
> meson_dw_hdmi module, component_add would call into
> try_to_bring_up_aggregate_device and find the unbound meson_drm aggregate
> device.
>
> This would in turn dereference some of the aggregate_device's struct
> entries which point to memory automatically freed by the devres API when
> unbinding the aggregate device from meson_drv_unbind, and trigger an
> use-after-free bug:
>
> [ +0.000014] =============================================================
> [ +0.000007] BUG: KASAN: use-after-free in find_components+0x468/0x500
> [ +0.000017] Read of size 8 at addr ffff000006731688 by task modprobe/2536
> [ +0.000018] CPU: 4 PID: 2536 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
> [ +0.000010] Hardware name: Hardkernel ODROID-N2Plus (DT)
> [ +0.000008] Call trace:
> [ +0.000005] dump_backtrace+0x1ec/0x280
> [ +0.000011] show_stack+0x24/0x80
> [ +0.000007] dump_stack_lvl+0x98/0xd4
> [ +0.000010] print_address_description.constprop.0+0x80/0x520
> [ +0.000011] print_report+0x128/0x260
> [ +0.000007] kasan_report+0xb8/0xfc
> [ +0.000007] __asan_report_load8_noabort+0x3c/0x50
> [ +0.000009] find_components+0x468/0x500
> [ +0.000008] try_to_bring_up_aggregate_device+0x64/0x390
> [ +0.000009] __component_add+0x1dc/0x49c
> [ +0.000009] component_add+0x20/0x30
> [ +0.000008] meson_dw_hdmi_probe+0x28/0x34 [meson_dw_hdmi]
> [ +0.000013] platform_probe+0xd0/0x220
> [ +0.000008] really_probe+0x3ac/0xa80
> [ +0.000008] __driver_probe_device+0x1f8/0x400
> [ +0.000008] driver_probe_device+0x68/0x1b0
> [ +0.000008] __driver_attach+0x20c/0x480
> [ +0.000009] bus_for_each_dev+0x114/0x1b0
> [ +0.000007] driver_attach+0x48/0x64
> [ +0.000009] bus_add_driver+0x390/0x564
> [ +0.000007] driver_register+0x1a8/0x3e4
> [ +0.000009] __platform_driver_register+0x6c/0x94
> [ +0.000007] meson_dw_hdmi_platform_driver_init+0x30/0x1000 [meson_dw_hdmi]
> [ +0.000014] do_one_initcall+0xc4/0x2b0
> [ +0.000008] do_init_module+0x154/0x570
> [ +0.000010] load_module+0x1a78/0x1ea4
> [ +0.000008] __do_sys_init_module+0x184/0x1cc
> [ +0.000008] __arm64_sys_init_module+0x78/0xb0
> [ +0.000008] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000009] do_el0_svc+0x50/0x70
> [ +0.000008] el0_svc+0x68/0x1a0
> [ +0.000009] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000014] Allocated by task 902:
> [ +0.000007] kasan_save_stack+0x2c/0x5c
> [ +0.000009] __kasan_kmalloc+0x90/0xd0
> [ +0.000007] __kmalloc_node+0x240/0x580
> [ +0.000010] memcg_alloc_slab_cgroups+0xa4/0x1ac
> [ +0.000010] memcg_slab_post_alloc_hook+0xbc/0x4c0
> [ +0.000008] kmem_cache_alloc_node+0x1d0/0x490
> [ +0.000009] __alloc_skb+0x1d4/0x310
> [ +0.000010] alloc_skb_with_frags+0x8c/0x620
> [ +0.000008] sock_alloc_send_pskb+0x5ac/0x6d0
> [ +0.000010] unix_dgram_sendmsg+0x2e0/0x12f0
> [ +0.000010] sock_sendmsg+0xcc/0x110
> [ +0.000007] sock_write_iter+0x1d0/0x304
> [ +0.000008] new_sync_write+0x364/0x460
> [ +0.000007] vfs_write+0x420/0x5ac
> [ +0.000008] ksys_write+0x19c/0x1f0
> [ +0.000008] __arm64_sys_write+0x78/0xb0
> [ +0.000007] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0x1a8/0x260
> [ +0.000009] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000008] el0t_64_sync_handler+0x11c/0x150
> [ +0.000008] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] Freed by task 2509:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000007] kasan_set_track+0x2c/0x40
> [ +0.000008] kasan_set_free_info+0x28/0x50
> [ +0.000008] ____kasan_slab_free+0x128/0x1d4
> [ +0.000008] __kasan_slab_free+0x18/0x24
> [ +0.000007] slab_free_freelist_hook+0x108/0x230
> [ +0.000010] kfree+0x110/0x35c
> [ +0.000008] release_nodes+0xf0/0x16c
> [ +0.000008] devres_release_all+0xfc/0x180
> [ +0.000008] device_unbind_cleanup+0x24/0x164
> [ +0.000008] device_release_driver_internal+0x3e8/0x5b0
> [ +0.000010] driver_detach+0xac/0x1b0
> [ +0.000008] bus_remove_driver+0x158/0x29c
> [ +0.000008] driver_unregister+0x70/0xb0
> [ +0.000009] platform_driver_unregister+0x20/0x2c
> [ +0.000007] 0xffff800003722d98
> [ +0.000012] __do_sys_delete_module+0x288/0x400
> [ +0.000009] __arm64_sys_delete_module+0x5c/0x80
> [ +0.000008] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000008] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] Last potentially related work creation:
> [ +0.000007] kasan_save_stack+0x2c/0x5c
> [ +0.000007] __kasan_record_aux_stack+0xb8/0xf0
> [ +0.000009] kasan_record_aux_stack_noalloc+0x14/0x20
> [ +0.000008] insert_work+0x54/0x290
> [ +0.000009] __queue_work+0x48c/0xd24
> [ +0.000008] queue_work_on+0x90/0x11c
> [ +0.000008] call_usermodehelper_exec+0x188/0x404
> [ +0.000010] kobject_uevent_env+0x5a8/0x794
> [ +0.000010] kobject_uevent+0x14/0x20
> [ +0.000008] driver_register+0x230/0x3e4
> [ +0.000009] __platform_driver_register+0x6c/0x94
> [ +0.000007] gxbb_driver_init+0x28/0x34
> [ +0.000010] do_one_initcall+0xc4/0x2b0
> [ +0.000008] do_initcalls+0x20c/0x24c
> [ +0.000010] kernel_init_freeable+0x22c/0x278
> [ +0.000009] kernel_init+0x3c/0x170
> [ +0.000008] ret_from_fork+0x10/0x20
>
> [ +0.000013] The buggy address belongs to the object at ffff000006731600
> which belongs to the cache kmalloc-256 of size 256
> [ +0.000009] The buggy address is located 136 bytes inside of
> 256-byte region [ffff000006731600, ffff000006731700)
>
> [ +0.000015] The buggy address belongs to the physical page:
> [ +0.000008] page:fffffc000019cc00 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff000006730a00 pfn:0x6730
> [ +0.000011] head:fffffc000019cc00 order:2 compound_mapcount:0 compound_pincount:0
> [ +0.000008] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
> [ +0.000016] raw: 0ffff00000010200 fffffc00000c3d08 fffffc0000ef2b08 ffff000000002680
> [ +0.000009] raw: ffff000006730a00 0000000000150014 00000001ffffffff 0000000000000000
> [ +0.000006] page dumped because: kasan: bad access detected
>
> [ +0.000011] Memory state around the buggy address:
> [ +0.000007] ffff000006731580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ +0.000007] ffff000006731600: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] >ffff000006731680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ^
> [ +0.000006] ffff000006731700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ +0.000007] ffff000006731780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [ +0.000006] ==================================================================
>
> Fix by adding 'remove' driver callback for meson-drm, and explicitly deleting the
> aggregate device.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 8da454a17b77..f3da1c214a7c 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -490,6 +490,13 @@ static int meson_drv_probe(struct platform_device *pdev)
> return 0;
> };
>
> +static int meson_drv_remove(struct platform_device *pdev)
> +{
> + component_master_del(&pdev->dev, &meson_drv_master_ops);
> +
> + return 0;
> +}
> +
> static struct meson_drm_match_data meson_drm_gxbb_data = {
> .compat = VPU_COMPATIBLE_GXBB,
> };
> @@ -527,6 +534,7 @@ static const struct dev_pm_ops meson_drv_pm_ops = {
>
> static struct platform_driver meson_drm_platform_driver = {
> .probe = meson_drv_probe,
> + .remove = meson_drv_remove,
> .shutdown = meson_drv_shutdown,
> .driver = {
> .name = "meson-drm",
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-19 1:09 [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Adrián Larumbe
2022-09-19 1:09 ` [PATCH 1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug Adrián Larumbe
2022-09-19 1:09 ` [PATCH 2/3] drm/meson: explicitly remove aggregate driver at module unload time Adrián Larumbe
@ 2022-09-19 1:09 ` Adrián Larumbe
2022-09-19 13:03 ` Neil Armstrong
2022-09-23 9:48 ` (subset) [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Neil Armstrong
3 siblings, 1 reply; 12+ messages in thread
From: Adrián Larumbe @ 2022-09-19 1:09 UTC (permalink / raw)
To: narmstrong, khilman, linux-amlogic, dri-devel; +Cc: adrian.larumbe
drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
were not manually removed at module unload time, which caused dangling
references to freed memory to remain linked in the global bridge_list.
When loading the driver modules back in, the same functions would again
call drm_bridge_add, and when traversing the global bridge_list, would
end up peeking into freed memory.
Once again KASAN revealed the problem:
[ +0.000095] =============================================================
[ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
[ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
[ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
[ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
[ +0.000008] Call trace:
[ +0.000006] dump_backtrace+0x1ec/0x280
[ +0.000012] show_stack+0x24/0x80
[ +0.000008] dump_stack_lvl+0x98/0xd4
[ +0.000011] print_address_description.constprop.0+0x80/0x520
[ +0.000011] print_report+0x128/0x260
[ +0.000008] kasan_report+0xb8/0xfc
[ +0.000008] __asan_report_load8_noabort+0x3c/0x50
[ +0.000009] __list_add_valid+0x9c/0x120
[ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
[ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
[ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
[ +0.000014] component_bind+0x174/0x520
[ +0.000012] component_bind_all+0x1a8/0x38c
[ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
[ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
[ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
[ +0.000010] component_master_add_with_match+0x1c8/0x284
[ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
[ +0.000026] platform_probe+0xd0/0x220
[ +0.000009] really_probe+0x3ac/0xa80
[ +0.000009] __driver_probe_device+0x1f8/0x400
[ +0.000009] driver_probe_device+0x68/0x1b0
[ +0.000009] __driver_attach+0x20c/0x480
[ +0.000008] bus_for_each_dev+0x114/0x1b0
[ +0.000009] driver_attach+0x48/0x64
[ +0.000008] bus_add_driver+0x390/0x564
[ +0.000009] driver_register+0x1a8/0x3e4
[ +0.000009] __platform_driver_register+0x6c/0x94
[ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
[ +0.000027] do_one_initcall+0xc4/0x2b0
[ +0.000011] do_init_module+0x154/0x570
[ +0.000011] load_module+0x1a78/0x1ea4
[ +0.000008] __do_sys_init_module+0x184/0x1cc
[ +0.000009] __arm64_sys_init_module+0x78/0xb0
[ +0.000009] invoke_syscall+0x74/0x260
[ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000012] el0t_64_sync_handler+0x11c/0x150
[ +0.000008] el0t_64_sync+0x18c/0x190
[ +0.000016] Allocated by task 879:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000011] __kasan_kmalloc+0x90/0xd0
[ +0.000007] __kmalloc+0x278/0x4a0
[ +0.000011] mpi_resize+0x13c/0x1d0
[ +0.000011] mpi_powm+0xd24/0x1570
[ +0.000009] rsa_enc+0x1a4/0x30c
[ +0.000009] pkcs1pad_verify+0x3f0/0x580
[ +0.000009] public_key_verify_signature+0x7a8/0xba4
[ +0.000010] public_key_verify_signature_2+0x40/0x60
[ +0.000008] verify_signature+0xb4/0x114
[ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
[ +0.000009] pkcs7_validate_trust+0xb8/0x15c
[ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
[ +0.000012] verify_pkcs7_signature+0x78/0xac
[ +0.000007] mod_verify_sig+0x110/0x190
[ +0.000009] module_sig_check+0x114/0x1e0
[ +0.000009] load_module+0xa0/0x1ea4
[ +0.000008] __do_sys_init_module+0x184/0x1cc
[ +0.000008] __arm64_sys_init_module+0x78/0xb0
[ +0.000008] invoke_syscall+0x74/0x260
[ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000009] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000013] Freed by task 2422:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000009] kasan_set_track+0x2c/0x40
[ +0.000007] kasan_set_free_info+0x28/0x50
[ +0.000009] ____kasan_slab_free+0x128/0x1d4
[ +0.000008] __kasan_slab_free+0x18/0x24
[ +0.000007] slab_free_freelist_hook+0x108/0x230
[ +0.000010] kfree+0x110/0x35c
[ +0.000008] release_nodes+0xf0/0x16c
[ +0.000009] devres_release_group+0x180/0x270
[ +0.000008] take_down_aggregate_device+0xcc/0x160
[ +0.000010] component_del+0x18c/0x360
[ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[ +0.000013] platform_remove+0x64/0xb0
[ +0.000008] device_remove+0xb8/0x154
[ +0.000009] device_release_driver_internal+0x398/0x5b0
[ +0.000009] driver_detach+0xac/0x1b0
[ +0.000009] bus_remove_driver+0x158/0x29c
[ +0.000008] driver_unregister+0x70/0xb0
[ +0.000009] platform_driver_unregister+0x20/0x2c
[ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
[ +0.000012] __do_sys_delete_module+0x288/0x400
[ +0.000009] __arm64_sys_delete_module+0x5c/0x80
[ +0.000009] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000008] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000013] The buggy address belongs to the object at ffff00003da29000
which belongs to the cache kmalloc-1k of size 1024
[ +0.000008] The buggy address is located 496 bytes inside of
1024-byte region [ffff00003da29000, ffff00003da29400)
[ +0.000015] The buggy address belongs to the physical page:
[ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
[ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
[ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
[ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
[ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
[ +0.000008] page dumped because: kasan: bad access detected
[ +0.000011] Memory state around the buggy address:
[ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ^
[ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ==================================================================
Fix by keeping track of which encoders were initialised in the meson_drm
structure and manually removing their bridges at aggregate driver's unbind
time.
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
drivers/gpu/drm/meson/meson_drv.c | 4 ++++
drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
drivers/gpu/drm/meson/meson_encoder_cvbs.c | 7 +++++++
drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 7 +++++++
drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
drivers/gpu/drm/meson/meson_venc.h | 15 +++++++++++++++
7 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index f3da1c214a7c..2b47154b73c2 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -387,6 +387,10 @@ static void meson_drv_unbind(struct device *dev)
drm_atomic_helper_shutdown(drm);
free_irq(priv->vsync_irq, drm);
drm_dev_put(drm);
+
+ meson_encoder_hdmi_remove(priv);
+ meson_encoder_cvbs_remove(priv);
+
component_unbind_all(dev, drm);
if (priv->afbcd.ops)
diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
index 177dac3ca3be..2cf279fb4f82 100644
--- a/drivers/gpu/drm/meson/meson_drv.h
+++ b/drivers/gpu/drm/meson/meson_drv.h
@@ -25,6 +25,12 @@ enum vpu_compatible {
VPU_COMPATIBLE_G12A = 3,
};
+enum {
+ MESON_ENC_CVBS = 0,
+ MESON_ENC_HDMI,
+ MESON_ENC_LAST,
+};
+
struct meson_drm_match_data {
enum vpu_compatible compat;
struct meson_afbcd_ops *afbcd_ops;
@@ -51,6 +57,7 @@ struct meson_drm {
struct drm_crtc *crtc;
struct drm_plane *primary_plane;
struct drm_plane *overlay_plane;
+ struct drm_encoder *encoders[MESON_ENC_LAST];
const struct meson_drm_soc_limits *limits;
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 8110a6e39320..00c958b08065 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -281,5 +281,12 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
}
drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
+ priv->encoders[MESON_ENC_CVBS] = &meson_encoder_cvbs->encoder;
+
return 0;
}
+
+void meson_encoder_cvbs_remove(struct meson_drm *priv)
+{
+ REMOVE_ENCODER_BRIDGES(cvbs, MESON_ENC_CVBS);
+}
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
index 61d9d183ce7f..09710fec3c66 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
@@ -25,5 +25,6 @@ struct meson_cvbs_mode {
extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
int meson_encoder_cvbs_init(struct meson_drm *priv);
+void meson_encoder_cvbs_remove(struct meson_drm *priv);
#endif /* __MESON_VENC_CVBS_H */
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 2f616c55c271..da6f2882cd97 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
meson_encoder_hdmi->cec_notifier = notifier;
}
+ priv->encoders[MESON_ENC_HDMI] = &meson_encoder_hdmi->encoder;
+
dev_dbg(priv->dev, "HDMI encoder initialized\n");
return 0;
@@ -460,3 +462,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
of_node_put(remote);
return ret;
}
+
+void meson_encoder_hdmi_remove(struct meson_drm *priv)
+{
+ REMOVE_ENCODER_BRIDGES(hdmi, MESON_ENC_HDMI);
+}
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
index ed19494f0956..a6cd38eb5f71 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
@@ -8,5 +8,6 @@
#define __MESON_ENCODER_HDMI_H
int meson_encoder_hdmi_init(struct meson_drm *priv);
+void meson_encoder_hdmi_remove(struct meson_drm *priv);
#endif /* __MESON_ENCODER_HDMI_H */
diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h
index 9138255ffc9e..9fc572860f8c 100644
--- a/drivers/gpu/drm/meson/meson_venc.h
+++ b/drivers/gpu/drm/meson/meson_venc.h
@@ -47,6 +47,21 @@ struct meson_cvbs_enci_mode {
unsigned int analog_sync_adj;
};
+#define REMOVE_ENCODER_BRIDGES(type, order) \
+{ \
+ struct meson_encoder_##type *meson_encoder_##type; \
+ struct drm_encoder *encoder; \
+ if (priv->encoders[order]) { \
+ encoder = priv->encoders[order]; \
+ meson_encoder_##type = \
+ container_of(encoder, \
+ struct meson_encoder_##type, \
+ encoder); \
+ drm_bridge_remove(&(meson_encoder_##type)->bridge); \
+ drm_bridge_remove((meson_encoder_##type)->next_bridge); \
+ } \
+} \
+
/* HDMI Clock parameters */
enum drm_mode_status
meson_venc_hdmi_supported_mode(const struct drm_display_mode *mode);
--
2.37.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-19 1:09 ` [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time Adrián Larumbe
@ 2022-09-19 13:03 ` Neil Armstrong
2022-09-20 11:49 ` Adrián Larumbe
0 siblings, 1 reply; 12+ messages in thread
From: Neil Armstrong @ 2022-09-19 13:03 UTC (permalink / raw)
To: Adrián Larumbe, narmstrong, khilman, linux-amlogic, dri-devel
On 19/09/2022 03:09, Adrián Larumbe wrote:
> drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
> were not manually removed at module unload time, which caused dangling
> references to freed memory to remain linked in the global bridge_list.
>
> When loading the driver modules back in, the same functions would again
> call drm_bridge_add, and when traversing the global bridge_list, would
> end up peeking into freed memory.
>
> Once again KASAN revealed the problem:
>
> [ +0.000095] =============================================================
> [ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
> [ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
>
> [ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
> [ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
> [ +0.000008] Call trace:
> [ +0.000006] dump_backtrace+0x1ec/0x280
> [ +0.000012] show_stack+0x24/0x80
> [ +0.000008] dump_stack_lvl+0x98/0xd4
> [ +0.000011] print_address_description.constprop.0+0x80/0x520
> [ +0.000011] print_report+0x128/0x260
> [ +0.000008] kasan_report+0xb8/0xfc
> [ +0.000008] __asan_report_load8_noabort+0x3c/0x50
> [ +0.000009] __list_add_valid+0x9c/0x120
> [ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
> [ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
> [ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
> [ +0.000014] component_bind+0x174/0x520
> [ +0.000012] component_bind_all+0x1a8/0x38c
> [ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
> [ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
> [ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
> [ +0.000010] component_master_add_with_match+0x1c8/0x284
> [ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
> [ +0.000026] platform_probe+0xd0/0x220
> [ +0.000009] really_probe+0x3ac/0xa80
> [ +0.000009] __driver_probe_device+0x1f8/0x400
> [ +0.000009] driver_probe_device+0x68/0x1b0
> [ +0.000009] __driver_attach+0x20c/0x480
> [ +0.000008] bus_for_each_dev+0x114/0x1b0
> [ +0.000009] driver_attach+0x48/0x64
> [ +0.000008] bus_add_driver+0x390/0x564
> [ +0.000009] driver_register+0x1a8/0x3e4
> [ +0.000009] __platform_driver_register+0x6c/0x94
> [ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
> [ +0.000027] do_one_initcall+0xc4/0x2b0
> [ +0.000011] do_init_module+0x154/0x570
> [ +0.000011] load_module+0x1a78/0x1ea4
> [ +0.000008] __do_sys_init_module+0x184/0x1cc
> [ +0.000009] __arm64_sys_init_module+0x78/0xb0
> [ +0.000009] invoke_syscall+0x74/0x260
> [ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000012] el0t_64_sync_handler+0x11c/0x150
> [ +0.000008] el0t_64_sync+0x18c/0x190
>
> [ +0.000016] Allocated by task 879:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000011] __kasan_kmalloc+0x90/0xd0
> [ +0.000007] __kmalloc+0x278/0x4a0
> [ +0.000011] mpi_resize+0x13c/0x1d0
> [ +0.000011] mpi_powm+0xd24/0x1570
> [ +0.000009] rsa_enc+0x1a4/0x30c
> [ +0.000009] pkcs1pad_verify+0x3f0/0x580
> [ +0.000009] public_key_verify_signature+0x7a8/0xba4
> [ +0.000010] public_key_verify_signature_2+0x40/0x60
> [ +0.000008] verify_signature+0xb4/0x114
> [ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
> [ +0.000009] pkcs7_validate_trust+0xb8/0x15c
> [ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
> [ +0.000012] verify_pkcs7_signature+0x78/0xac
> [ +0.000007] mod_verify_sig+0x110/0x190
> [ +0.000009] module_sig_check+0x114/0x1e0
> [ +0.000009] load_module+0xa0/0x1ea4
> [ +0.000008] __do_sys_init_module+0x184/0x1cc
> [ +0.000008] __arm64_sys_init_module+0x78/0xb0
> [ +0.000008] invoke_syscall+0x74/0x260
> [ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000009] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] Freed by task 2422:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000009] kasan_set_track+0x2c/0x40
> [ +0.000007] kasan_set_free_info+0x28/0x50
> [ +0.000009] ____kasan_slab_free+0x128/0x1d4
> [ +0.000008] __kasan_slab_free+0x18/0x24
> [ +0.000007] slab_free_freelist_hook+0x108/0x230
> [ +0.000010] kfree+0x110/0x35c
> [ +0.000008] release_nodes+0xf0/0x16c
> [ +0.000009] devres_release_group+0x180/0x270
> [ +0.000008] take_down_aggregate_device+0xcc/0x160
> [ +0.000010] component_del+0x18c/0x360
> [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
> [ +0.000013] platform_remove+0x64/0xb0
> [ +0.000008] device_remove+0xb8/0x154
> [ +0.000009] device_release_driver_internal+0x398/0x5b0
> [ +0.000009] driver_detach+0xac/0x1b0
> [ +0.000009] bus_remove_driver+0x158/0x29c
> [ +0.000008] driver_unregister+0x70/0xb0
> [ +0.000009] platform_driver_unregister+0x20/0x2c
> [ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
> [ +0.000012] __do_sys_delete_module+0x288/0x400
> [ +0.000009] __arm64_sys_delete_module+0x5c/0x80
> [ +0.000009] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000008] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] The buggy address belongs to the object at ffff00003da29000
> which belongs to the cache kmalloc-1k of size 1024
> [ +0.000008] The buggy address is located 496 bytes inside of
> 1024-byte region [ffff00003da29000, ffff00003da29400)
>
> [ +0.000015] The buggy address belongs to the physical page:
> [ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
> [ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
> [ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
> [ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
> [ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
> [ +0.000008] page dumped because: kasan: bad access detected
>
> [ +0.000011] Memory state around the buggy address:
> [ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ^
> [ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ==================================================================
>
> Fix by keeping track of which encoders were initialised in the meson_drm
> structure and manually removing their bridges at aggregate driver's unbind
> time.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 4 ++++
> drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
> drivers/gpu/drm/meson/meson_encoder_cvbs.c | 7 +++++++
> drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 7 +++++++
> drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
> drivers/gpu/drm/meson/meson_venc.h | 15 +++++++++++++++
> 7 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index f3da1c214a7c..2b47154b73c2 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -387,6 +387,10 @@ static void meson_drv_unbind(struct device *dev)
> drm_atomic_helper_shutdown(drm);
> free_irq(priv->vsync_irq, drm);
> drm_dev_put(drm);
> +
> + meson_encoder_hdmi_remove(priv);
> + meson_encoder_cvbs_remove(priv);
> +
> component_unbind_all(dev, drm);
>
> if (priv->afbcd.ops)
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 177dac3ca3be..2cf279fb4f82 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -25,6 +25,12 @@ enum vpu_compatible {
> VPU_COMPATIBLE_G12A = 3,
> };
>
> +enum {
> + MESON_ENC_CVBS = 0,
> + MESON_ENC_HDMI,
> + MESON_ENC_LAST,
> +};
> +
> struct meson_drm_match_data {
> enum vpu_compatible compat;
> struct meson_afbcd_ops *afbcd_ops;
> @@ -51,6 +57,7 @@ struct meson_drm {
> struct drm_crtc *crtc;
> struct drm_plane *primary_plane;
> struct drm_plane *overlay_plane;
> + struct drm_encoder *encoders[MESON_ENC_LAST];
>
> const struct meson_drm_soc_limits *limits;
>
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> index 8110a6e39320..00c958b08065 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> @@ -281,5 +281,12 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
> }
> drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
>
> + priv->encoders[MESON_ENC_CVBS] = &meson_encoder_cvbs->encoder;
> +
> return 0;
> }
> +
> +void meson_encoder_cvbs_remove(struct meson_drm *priv)
> +{
> + REMOVE_ENCODER_BRIDGES(cvbs, MESON_ENC_CVBS);
> +}
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> index 61d9d183ce7f..09710fec3c66 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> @@ -25,5 +25,6 @@ struct meson_cvbs_mode {
> extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
>
> int meson_encoder_cvbs_init(struct meson_drm *priv);
> +void meson_encoder_cvbs_remove(struct meson_drm *priv);
>
> #endif /* __MESON_VENC_CVBS_H */
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 2f616c55c271..da6f2882cd97 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> meson_encoder_hdmi->cec_notifier = notifier;
> }
>
> + priv->encoders[MESON_ENC_HDMI] = &meson_encoder_hdmi->encoder;
> +
> dev_dbg(priv->dev, "HDMI encoder initialized\n");
>
> return 0;
> @@ -460,3 +462,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> of_node_put(remote);
> return ret;
> }
> +
> +void meson_encoder_hdmi_remove(struct meson_drm *priv)
> +{
> + REMOVE_ENCODER_BRIDGES(hdmi, MESON_ENC_HDMI);
> +}
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> index ed19494f0956..a6cd38eb5f71 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> @@ -8,5 +8,6 @@
> #define __MESON_ENCODER_HDMI_H
>
> int meson_encoder_hdmi_init(struct meson_drm *priv);
> +void meson_encoder_hdmi_remove(struct meson_drm *priv);
>
> #endif /* __MESON_ENCODER_HDMI_H */
> diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h
> index 9138255ffc9e..9fc572860f8c 100644
> --- a/drivers/gpu/drm/meson/meson_venc.h
> +++ b/drivers/gpu/drm/meson/meson_venc.h
> @@ -47,6 +47,21 @@ struct meson_cvbs_enci_mode {
> unsigned int analog_sync_adj;
> };
>
> +#define REMOVE_ENCODER_BRIDGES(type, order) \
> +{ \
> + struct meson_encoder_##type *meson_encoder_##type; \
> + struct drm_encoder *encoder; \
> + if (priv->encoders[order]) { \
> + encoder = priv->encoders[order]; \
> + meson_encoder_##type = \
> + container_of(encoder, \
> + struct meson_encoder_##type, \
> + encoder); \
> + drm_bridge_remove(&(meson_encoder_##type)->bridge); \
> + drm_bridge_remove((meson_encoder_##type)->next_bridge); \
> + } \
> +}
This looks over-complicated, why not store the "struct meson_encoder_cvbs/_hdmi" raw pointer in the struct meson_drm encoders table instead ?
With this no need to use container_of and directly call drm_bridge_remove() on bridge & next_bridge.
\
> +
> /* HDMI Clock parameters */
> enum drm_mode_status
> meson_venc_hdmi_supported_mode(const struct drm_display_mode *mode);
Thanks,
Neil
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-19 13:03 ` Neil Armstrong
@ 2022-09-20 11:49 ` Adrián Larumbe
2022-09-20 12:56 ` Neil Armstrong
0 siblings, 1 reply; 12+ messages in thread
From: Adrián Larumbe @ 2022-09-20 11:49 UTC (permalink / raw)
To: Neil Armstrong; +Cc: narmstrong, khilman, linux-amlogic, dri-devel
On 19.09.2022 15:03, Neil Armstrong wrote:
> On 19/09/2022 03:09, Adrián Larumbe wrote:
> > drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
> > were not manually removed at module unload time, which caused dangling
> > references to freed memory to remain linked in the global bridge_list.
> >
> > When loading the driver modules back in, the same functions would again
> > call drm_bridge_add, and when traversing the global bridge_list, would
> > end up peeking into freed memory.
> >
> > Once again KASAN revealed the problem:
> >
> > [ +0.000095] =============================================================
> > [ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
> > [ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
> >
> > [ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
> > [ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
> > [ +0.000008] Call trace:
> > [ +0.000006] dump_backtrace+0x1ec/0x280
> > [ +0.000012] show_stack+0x24/0x80
> > [ +0.000008] dump_stack_lvl+0x98/0xd4
> > [ +0.000011] print_address_description.constprop.0+0x80/0x520
> > [ +0.000011] print_report+0x128/0x260
> > [ +0.000008] kasan_report+0xb8/0xfc
> > [ +0.000008] __asan_report_load8_noabort+0x3c/0x50
> > [ +0.000009] __list_add_valid+0x9c/0x120
> > [ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
> > [ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
> > [ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
> > [ +0.000014] component_bind+0x174/0x520
> > [ +0.000012] component_bind_all+0x1a8/0x38c
> > [ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
> > [ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
> > [ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
> > [ +0.000010] component_master_add_with_match+0x1c8/0x284
> > [ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
> > [ +0.000026] platform_probe+0xd0/0x220
> > [ +0.000009] really_probe+0x3ac/0xa80
> > [ +0.000009] __driver_probe_device+0x1f8/0x400
> > [ +0.000009] driver_probe_device+0x68/0x1b0
> > [ +0.000009] __driver_attach+0x20c/0x480
> > [ +0.000008] bus_for_each_dev+0x114/0x1b0
> > [ +0.000009] driver_attach+0x48/0x64
> > [ +0.000008] bus_add_driver+0x390/0x564
> > [ +0.000009] driver_register+0x1a8/0x3e4
> > [ +0.000009] __platform_driver_register+0x6c/0x94
> > [ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
> > [ +0.000027] do_one_initcall+0xc4/0x2b0
> > [ +0.000011] do_init_module+0x154/0x570
> > [ +0.000011] load_module+0x1a78/0x1ea4
> > [ +0.000008] __do_sys_init_module+0x184/0x1cc
> > [ +0.000009] __arm64_sys_init_module+0x78/0xb0
> > [ +0.000009] invoke_syscall+0x74/0x260
> > [ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
> > [ +0.000008] do_el0_svc+0x50/0x70
> > [ +0.000007] el0_svc+0x68/0x1a0
> > [ +0.000012] el0t_64_sync_handler+0x11c/0x150
> > [ +0.000008] el0t_64_sync+0x18c/0x190
> >
> > [ +0.000016] Allocated by task 879:
> > [ +0.000008] kasan_save_stack+0x2c/0x5c
> > [ +0.000011] __kasan_kmalloc+0x90/0xd0
> > [ +0.000007] __kmalloc+0x278/0x4a0
> > [ +0.000011] mpi_resize+0x13c/0x1d0
> > [ +0.000011] mpi_powm+0xd24/0x1570
> > [ +0.000009] rsa_enc+0x1a4/0x30c
> > [ +0.000009] pkcs1pad_verify+0x3f0/0x580
> > [ +0.000009] public_key_verify_signature+0x7a8/0xba4
> > [ +0.000010] public_key_verify_signature_2+0x40/0x60
> > [ +0.000008] verify_signature+0xb4/0x114
> > [ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
> > [ +0.000009] pkcs7_validate_trust+0xb8/0x15c
> > [ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
> > [ +0.000012] verify_pkcs7_signature+0x78/0xac
> > [ +0.000007] mod_verify_sig+0x110/0x190
> > [ +0.000009] module_sig_check+0x114/0x1e0
> > [ +0.000009] load_module+0xa0/0x1ea4
> > [ +0.000008] __do_sys_init_module+0x184/0x1cc
> > [ +0.000008] __arm64_sys_init_module+0x78/0xb0
> > [ +0.000008] invoke_syscall+0x74/0x260
> > [ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
> > [ +0.000008] do_el0_svc+0x50/0x70
> > [ +0.000007] el0_svc+0x68/0x1a0
> > [ +0.000009] el0t_64_sync_handler+0x11c/0x150
> > [ +0.000009] el0t_64_sync+0x18c/0x190
> >
> > [ +0.000013] Freed by task 2422:
> > [ +0.000008] kasan_save_stack+0x2c/0x5c
> > [ +0.000009] kasan_set_track+0x2c/0x40
> > [ +0.000007] kasan_set_free_info+0x28/0x50
> > [ +0.000009] ____kasan_slab_free+0x128/0x1d4
> > [ +0.000008] __kasan_slab_free+0x18/0x24
> > [ +0.000007] slab_free_freelist_hook+0x108/0x230
> > [ +0.000010] kfree+0x110/0x35c
> > [ +0.000008] release_nodes+0xf0/0x16c
> > [ +0.000009] devres_release_group+0x180/0x270
> > [ +0.000008] take_down_aggregate_device+0xcc/0x160
> > [ +0.000010] component_del+0x18c/0x360
> > [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
> > [ +0.000013] platform_remove+0x64/0xb0
> > [ +0.000008] device_remove+0xb8/0x154
> > [ +0.000009] device_release_driver_internal+0x398/0x5b0
> > [ +0.000009] driver_detach+0xac/0x1b0
> > [ +0.000009] bus_remove_driver+0x158/0x29c
> > [ +0.000008] driver_unregister+0x70/0xb0
> > [ +0.000009] platform_driver_unregister+0x20/0x2c
> > [ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
> > [ +0.000012] __do_sys_delete_module+0x288/0x400
> > [ +0.000009] __arm64_sys_delete_module+0x5c/0x80
> > [ +0.000009] invoke_syscall+0x74/0x260
> > [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> > [ +0.000008] do_el0_svc+0x50/0x70
> > [ +0.000007] el0_svc+0x68/0x1a0
> > [ +0.000008] el0t_64_sync_handler+0x11c/0x150
> > [ +0.000009] el0t_64_sync+0x18c/0x190
> >
> > [ +0.000013] The buggy address belongs to the object at ffff00003da29000
> > which belongs to the cache kmalloc-1k of size 1024
> > [ +0.000008] The buggy address is located 496 bytes inside of
> > 1024-byte region [ffff00003da29000, ffff00003da29400)
> >
> > [ +0.000015] The buggy address belongs to the physical page:
> > [ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
> > [ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
> > [ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
> > [ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
> > [ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
> > [ +0.000008] page dumped because: kasan: bad access detected
> >
> > [ +0.000011] Memory state around the buggy address:
> > [ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [ +0.000007] ^
> > [ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > [ +0.000007] ==================================================================
> >
> > Fix by keeping track of which encoders were initialised in the meson_drm
> > structure and manually removing their bridges at aggregate driver's unbind
> > time.
> >
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > ---
> > drivers/gpu/drm/meson/meson_drv.c | 4 ++++
> > drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
> > drivers/gpu/drm/meson/meson_encoder_cvbs.c | 7 +++++++
> > drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
> > drivers/gpu/drm/meson/meson_encoder_hdmi.c | 7 +++++++
> > drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
> > drivers/gpu/drm/meson/meson_venc.h | 15 +++++++++++++++
> > 7 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> > index f3da1c214a7c..2b47154b73c2 100644
> > --- a/drivers/gpu/drm/meson/meson_drv.c
> > +++ b/drivers/gpu/drm/meson/meson_drv.c
> > @@ -387,6 +387,10 @@ static void meson_drv_unbind(struct device *dev)
> > drm_atomic_helper_shutdown(drm);
> > free_irq(priv->vsync_irq, drm);
> > drm_dev_put(drm);
> > +
> > + meson_encoder_hdmi_remove(priv);
> > + meson_encoder_cvbs_remove(priv);
> > +
> > component_unbind_all(dev, drm);
> > if (priv->afbcd.ops)
> > diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> > index 177dac3ca3be..2cf279fb4f82 100644
> > --- a/drivers/gpu/drm/meson/meson_drv.h
> > +++ b/drivers/gpu/drm/meson/meson_drv.h
> > @@ -25,6 +25,12 @@ enum vpu_compatible {
> > VPU_COMPATIBLE_G12A = 3,
> > };
> > +enum {
> > + MESON_ENC_CVBS = 0,
> > + MESON_ENC_HDMI,
> > + MESON_ENC_LAST,
> > +};
> > +
> > struct meson_drm_match_data {
> > enum vpu_compatible compat;
> > struct meson_afbcd_ops *afbcd_ops;
> > @@ -51,6 +57,7 @@ struct meson_drm {
> > struct drm_crtc *crtc;
> > struct drm_plane *primary_plane;
> > struct drm_plane *overlay_plane;
> > + struct drm_encoder *encoders[MESON_ENC_LAST];
> > const struct meson_drm_soc_limits *limits;
> > diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> > index 8110a6e39320..00c958b08065 100644
> > --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> > +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> > @@ -281,5 +281,12 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
> > }
> > drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
> > + priv->encoders[MESON_ENC_CVBS] = &meson_encoder_cvbs->encoder;
> > +
> > return 0;
> > }
> > +
> > +void meson_encoder_cvbs_remove(struct meson_drm *priv)
> > +{
> > + REMOVE_ENCODER_BRIDGES(cvbs, MESON_ENC_CVBS);
> > +}
> > diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> > index 61d9d183ce7f..09710fec3c66 100644
> > --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> > +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> > @@ -25,5 +25,6 @@ struct meson_cvbs_mode {
> > extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
> > int meson_encoder_cvbs_init(struct meson_drm *priv);
> > +void meson_encoder_cvbs_remove(struct meson_drm *priv);
> > #endif /* __MESON_VENC_CVBS_H */
> > diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> > index 2f616c55c271..da6f2882cd97 100644
> > --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> > +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> > @@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> > meson_encoder_hdmi->cec_notifier = notifier;
> > }
> > + priv->encoders[MESON_ENC_HDMI] = &meson_encoder_hdmi->encoder;
> > +
> > dev_dbg(priv->dev, "HDMI encoder initialized\n");
> > return 0;
> > @@ -460,3 +462,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> > of_node_put(remote);
> > return ret;
> > }
> > +
> > +void meson_encoder_hdmi_remove(struct meson_drm *priv)
> > +{
> > + REMOVE_ENCODER_BRIDGES(hdmi, MESON_ENC_HDMI);
> > +}
> > diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> > index ed19494f0956..a6cd38eb5f71 100644
> > --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> > +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> > @@ -8,5 +8,6 @@
> > #define __MESON_ENCODER_HDMI_H
> > int meson_encoder_hdmi_init(struct meson_drm *priv);
> > +void meson_encoder_hdmi_remove(struct meson_drm *priv);
> > #endif /* __MESON_ENCODER_HDMI_H */
> > diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h
> > index 9138255ffc9e..9fc572860f8c 100644
> > --- a/drivers/gpu/drm/meson/meson_venc.h
> > +++ b/drivers/gpu/drm/meson/meson_venc.h
> > @@ -47,6 +47,21 @@ struct meson_cvbs_enci_mode {
> > unsigned int analog_sync_adj;
> > };
> > +#define REMOVE_ENCODER_BRIDGES(type, order) \
> > +{ \
> > + struct meson_encoder_##type *meson_encoder_##type; \
> > + struct drm_encoder *encoder; \
> > + if (priv->encoders[order]) { \
> > + encoder = priv->encoders[order]; \
> > + meson_encoder_##type = \
> > + container_of(encoder, \
> > + struct meson_encoder_##type, \
> > + encoder); \
> > + drm_bridge_remove(&(meson_encoder_##type)->bridge); \
> > + drm_bridge_remove((meson_encoder_##type)->next_bridge); \
> > + } \
> > +}
>
>
> This looks over-complicated, why not store the "struct meson_encoder_cvbs/_hdmi" raw pointer in the struct meson_drm encoders table instead ?
>
> With this no need to use container_of and directly call drm_bridge_remove() on bridge & next_bridge.
Would it be alright if I store them as void pointers and cast them back to the
right struct type in the corresponding encoder remove function?
Or maybe keeping two separate struct pointer of the specific type rather than an
array of generic ones.
Also I thought abstracting the encoder type might spare us one forward
declaration in meson_drm.h, but maybe that's not a big deal.
I'm thinking that this means Meson VPU won't ever need to support new types of
encoders, so a bit of code repetition in the encoder's bridge remove function is
alright.
> > +
> > /* HDMI Clock parameters */
> > enum drm_mode_status
> > meson_venc_hdmi_supported_mode(const struct drm_display_mode *mode);
>
> Thanks,
> Neil
Adrian
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-20 11:49 ` Adrián Larumbe
@ 2022-09-20 12:56 ` Neil Armstrong
2022-09-20 22:28 ` [PATCH v2 " Adrián Larumbe
0 siblings, 1 reply; 12+ messages in thread
From: Neil Armstrong @ 2022-09-20 12:56 UTC (permalink / raw)
To: Adrián Larumbe; +Cc: khilman, linux-amlogic, dri-devel, narmstrong
On 20/09/2022 13:49, Adrián Larumbe wrote:
> On 19.09.2022 15:03, Neil Armstrong wrote:
>> On 19/09/2022 03:09, Adrián Larumbe wrote:
>>> drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
>>> were not manually removed at module unload time, which caused dangling
>>> references to freed memory to remain linked in the global bridge_list.
>>>
>>> When loading the driver modules back in, the same functions would again
>>> call drm_bridge_add, and when traversing the global bridge_list, would
>>> end up peeking into freed memory.
>>>
>>> Once again KASAN revealed the problem:
>>>
>>> [ +0.000095] =============================================================
>>> [ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
>>> [ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
>>>
>>> [ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
>>> [ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
>>> [ +0.000008] Call trace:
>>> [ +0.000006] dump_backtrace+0x1ec/0x280
>>> [ +0.000012] show_stack+0x24/0x80
>>> [ +0.000008] dump_stack_lvl+0x98/0xd4
>>> [ +0.000011] print_address_description.constprop.0+0x80/0x520
>>> [ +0.000011] print_report+0x128/0x260
>>> [ +0.000008] kasan_report+0xb8/0xfc
>>> [ +0.000008] __asan_report_load8_noabort+0x3c/0x50
>>> [ +0.000009] __list_add_valid+0x9c/0x120
>>> [ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
>>> [ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
>>> [ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
>>> [ +0.000014] component_bind+0x174/0x520
>>> [ +0.000012] component_bind_all+0x1a8/0x38c
>>> [ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
>>> [ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
>>> [ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
>>> [ +0.000010] component_master_add_with_match+0x1c8/0x284
>>> [ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
>>> [ +0.000026] platform_probe+0xd0/0x220
>>> [ +0.000009] really_probe+0x3ac/0xa80
>>> [ +0.000009] __driver_probe_device+0x1f8/0x400
>>> [ +0.000009] driver_probe_device+0x68/0x1b0
>>> [ +0.000009] __driver_attach+0x20c/0x480
>>> [ +0.000008] bus_for_each_dev+0x114/0x1b0
>>> [ +0.000009] driver_attach+0x48/0x64
>>> [ +0.000008] bus_add_driver+0x390/0x564
>>> [ +0.000009] driver_register+0x1a8/0x3e4
>>> [ +0.000009] __platform_driver_register+0x6c/0x94
>>> [ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
>>> [ +0.000027] do_one_initcall+0xc4/0x2b0
>>> [ +0.000011] do_init_module+0x154/0x570
>>> [ +0.000011] load_module+0x1a78/0x1ea4
>>> [ +0.000008] __do_sys_init_module+0x184/0x1cc
>>> [ +0.000009] __arm64_sys_init_module+0x78/0xb0
>>> [ +0.000009] invoke_syscall+0x74/0x260
>>> [ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
>>> [ +0.000008] do_el0_svc+0x50/0x70
>>> [ +0.000007] el0_svc+0x68/0x1a0
>>> [ +0.000012] el0t_64_sync_handler+0x11c/0x150
>>> [ +0.000008] el0t_64_sync+0x18c/0x190
>>>
>>> [ +0.000016] Allocated by task 879:
>>> [ +0.000008] kasan_save_stack+0x2c/0x5c
>>> [ +0.000011] __kasan_kmalloc+0x90/0xd0
>>> [ +0.000007] __kmalloc+0x278/0x4a0
>>> [ +0.000011] mpi_resize+0x13c/0x1d0
>>> [ +0.000011] mpi_powm+0xd24/0x1570
>>> [ +0.000009] rsa_enc+0x1a4/0x30c
>>> [ +0.000009] pkcs1pad_verify+0x3f0/0x580
>>> [ +0.000009] public_key_verify_signature+0x7a8/0xba4
>>> [ +0.000010] public_key_verify_signature_2+0x40/0x60
>>> [ +0.000008] verify_signature+0xb4/0x114
>>> [ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
>>> [ +0.000009] pkcs7_validate_trust+0xb8/0x15c
>>> [ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
>>> [ +0.000012] verify_pkcs7_signature+0x78/0xac
>>> [ +0.000007] mod_verify_sig+0x110/0x190
>>> [ +0.000009] module_sig_check+0x114/0x1e0
>>> [ +0.000009] load_module+0xa0/0x1ea4
>>> [ +0.000008] __do_sys_init_module+0x184/0x1cc
>>> [ +0.000008] __arm64_sys_init_module+0x78/0xb0
>>> [ +0.000008] invoke_syscall+0x74/0x260
>>> [ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
>>> [ +0.000008] do_el0_svc+0x50/0x70
>>> [ +0.000007] el0_svc+0x68/0x1a0
>>> [ +0.000009] el0t_64_sync_handler+0x11c/0x150
>>> [ +0.000009] el0t_64_sync+0x18c/0x190
>>>
>>> [ +0.000013] Freed by task 2422:
>>> [ +0.000008] kasan_save_stack+0x2c/0x5c
>>> [ +0.000009] kasan_set_track+0x2c/0x40
>>> [ +0.000007] kasan_set_free_info+0x28/0x50
>>> [ +0.000009] ____kasan_slab_free+0x128/0x1d4
>>> [ +0.000008] __kasan_slab_free+0x18/0x24
>>> [ +0.000007] slab_free_freelist_hook+0x108/0x230
>>> [ +0.000010] kfree+0x110/0x35c
>>> [ +0.000008] release_nodes+0xf0/0x16c
>>> [ +0.000009] devres_release_group+0x180/0x270
>>> [ +0.000008] take_down_aggregate_device+0xcc/0x160
>>> [ +0.000010] component_del+0x18c/0x360
>>> [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
>>> [ +0.000013] platform_remove+0x64/0xb0
>>> [ +0.000008] device_remove+0xb8/0x154
>>> [ +0.000009] device_release_driver_internal+0x398/0x5b0
>>> [ +0.000009] driver_detach+0xac/0x1b0
>>> [ +0.000009] bus_remove_driver+0x158/0x29c
>>> [ +0.000008] driver_unregister+0x70/0xb0
>>> [ +0.000009] platform_driver_unregister+0x20/0x2c
>>> [ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
>>> [ +0.000012] __do_sys_delete_module+0x288/0x400
>>> [ +0.000009] __arm64_sys_delete_module+0x5c/0x80
>>> [ +0.000009] invoke_syscall+0x74/0x260
>>> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
>>> [ +0.000008] do_el0_svc+0x50/0x70
>>> [ +0.000007] el0_svc+0x68/0x1a0
>>> [ +0.000008] el0t_64_sync_handler+0x11c/0x150
>>> [ +0.000009] el0t_64_sync+0x18c/0x190
>>>
>>> [ +0.000013] The buggy address belongs to the object at ffff00003da29000
>>> which belongs to the cache kmalloc-1k of size 1024
>>> [ +0.000008] The buggy address is located 496 bytes inside of
>>> 1024-byte region [ffff00003da29000, ffff00003da29400)
>>>
>>> [ +0.000015] The buggy address belongs to the physical page:
>>> [ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
>>> [ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
>>> [ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
>>> [ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
>>> [ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
>>> [ +0.000008] page dumped because: kasan: bad access detected
>>>
>>> [ +0.000011] Memory state around the buggy address:
>>> [ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>> [ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>> [ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>> [ +0.000007] ^
>>> [ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>> [ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>> [ +0.000007] ==================================================================
>>>
>>> Fix by keeping track of which encoders were initialised in the meson_drm
>>> structure and manually removing their bridges at aggregate driver's unbind
>>> time.
>>>
>>> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
>>> ---
>>> drivers/gpu/drm/meson/meson_drv.c | 4 ++++
>>> drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
>>> drivers/gpu/drm/meson/meson_encoder_cvbs.c | 7 +++++++
>>> drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
>>> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 7 +++++++
>>> drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
>>> drivers/gpu/drm/meson/meson_venc.h | 15 +++++++++++++++
>>> 7 files changed, 42 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>>> index f3da1c214a7c..2b47154b73c2 100644
>>> --- a/drivers/gpu/drm/meson/meson_drv.c
>>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>>> @@ -387,6 +387,10 @@ static void meson_drv_unbind(struct device *dev)
>>> drm_atomic_helper_shutdown(drm);
>>> free_irq(priv->vsync_irq, drm);
>>> drm_dev_put(drm);
>>> +
>>> + meson_encoder_hdmi_remove(priv);
>>> + meson_encoder_cvbs_remove(priv);
>>> +
>>> component_unbind_all(dev, drm);
>>> if (priv->afbcd.ops)
>>> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
>>> index 177dac3ca3be..2cf279fb4f82 100644
>>> --- a/drivers/gpu/drm/meson/meson_drv.h
>>> +++ b/drivers/gpu/drm/meson/meson_drv.h
>>> @@ -25,6 +25,12 @@ enum vpu_compatible {
>>> VPU_COMPATIBLE_G12A = 3,
>>> };
>>> +enum {
>>> + MESON_ENC_CVBS = 0,
>>> + MESON_ENC_HDMI,
>>> + MESON_ENC_LAST,
>>> +};
>>> +
>>> struct meson_drm_match_data {
>>> enum vpu_compatible compat;
>>> struct meson_afbcd_ops *afbcd_ops;
>>> @@ -51,6 +57,7 @@ struct meson_drm {
>>> struct drm_crtc *crtc;
>>> struct drm_plane *primary_plane;
>>> struct drm_plane *overlay_plane;
>>> + struct drm_encoder *encoders[MESON_ENC_LAST];
>>> const struct meson_drm_soc_limits *limits;
>>> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>>> index 8110a6e39320..00c958b08065 100644
>>> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>>> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>>> @@ -281,5 +281,12 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
>>> }
>>> drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
>>> + priv->encoders[MESON_ENC_CVBS] = &meson_encoder_cvbs->encoder;
>>> +
>>> return 0;
>>> }
>>> +
>>> +void meson_encoder_cvbs_remove(struct meson_drm *priv)
>>> +{
>>> + REMOVE_ENCODER_BRIDGES(cvbs, MESON_ENC_CVBS);
>>> +}
>>> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>>> index 61d9d183ce7f..09710fec3c66 100644
>>> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>>> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
>>> @@ -25,5 +25,6 @@ struct meson_cvbs_mode {
>>> extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
>>> int meson_encoder_cvbs_init(struct meson_drm *priv);
>>> +void meson_encoder_cvbs_remove(struct meson_drm *priv);
>>> #endif /* __MESON_VENC_CVBS_H */
>>> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>>> index 2f616c55c271..da6f2882cd97 100644
>>> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>>> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>>> @@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
>>> meson_encoder_hdmi->cec_notifier = notifier;
>>> }
>>> + priv->encoders[MESON_ENC_HDMI] = &meson_encoder_hdmi->encoder;
>>> +
>>> dev_dbg(priv->dev, "HDMI encoder initialized\n");
>>> return 0;
>>> @@ -460,3 +462,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
>>> of_node_put(remote);
>>> return ret;
>>> }
>>> +
>>> +void meson_encoder_hdmi_remove(struct meson_drm *priv)
>>> +{
>>> + REMOVE_ENCODER_BRIDGES(hdmi, MESON_ENC_HDMI);
>>> +}
>>> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
>>> index ed19494f0956..a6cd38eb5f71 100644
>>> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
>>> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
>>> @@ -8,5 +8,6 @@
>>> #define __MESON_ENCODER_HDMI_H
>>> int meson_encoder_hdmi_init(struct meson_drm *priv);
>>> +void meson_encoder_hdmi_remove(struct meson_drm *priv);
>>> #endif /* __MESON_ENCODER_HDMI_H */
>>> diff --git a/drivers/gpu/drm/meson/meson_venc.h b/drivers/gpu/drm/meson/meson_venc.h
>>> index 9138255ffc9e..9fc572860f8c 100644
>>> --- a/drivers/gpu/drm/meson/meson_venc.h
>>> +++ b/drivers/gpu/drm/meson/meson_venc.h
>>> @@ -47,6 +47,21 @@ struct meson_cvbs_enci_mode {
>>> unsigned int analog_sync_adj;
>>> };
>>> +#define REMOVE_ENCODER_BRIDGES(type, order) \
>>> +{ \
>>> + struct meson_encoder_##type *meson_encoder_##type; \
>>> + struct drm_encoder *encoder; \
>>> + if (priv->encoders[order]) { \
>>> + encoder = priv->encoders[order]; \
>>> + meson_encoder_##type = \
>>> + container_of(encoder, \
>>> + struct meson_encoder_##type, \
>>> + encoder); \
>>> + drm_bridge_remove(&(meson_encoder_##type)->bridge); \
>>> + drm_bridge_remove((meson_encoder_##type)->next_bridge); \
>>> + } \
>>> +}
>>
>>
>> This looks over-complicated, why not store the "struct meson_encoder_cvbs/_hdmi" raw pointer in the struct meson_drm encoders table instead ?
>>
>> With this no need to use container_of and directly call drm_bridge_remove() on bridge & next_bridge.
>
> Would it be alright if I store them as void pointers and cast them back to the
> right struct type in the corresponding encoder remove function?
> Or maybe keeping two separate struct pointer of the specific type rather than an
> array of generic ones.
Yes it's ok to store them as void pointer and cast them back.
>
> Also I thought abstracting the encoder type might spare us one forward
> declaration in meson_drm.h, but maybe that's not a big deal.
> I'm thinking that this means Meson VPU won't ever need to support new types of
> encoders, so a bit of code repetition in the encoder's bridge remove function is
> alright.
Yes and no, a DSI support is in review and new SoCs will support DisplayPort output...
>
>>> +
>>> /* HDMI Clock parameters */
>>> enum drm_mode_status
>>> meson_venc_hdmi_supported_mode(const struct drm_display_mode *mode);
>>
>> Thanks,
>> Neil
>
> Adrian
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-20 12:56 ` Neil Armstrong
@ 2022-09-20 22:28 ` Adrián Larumbe
2022-09-23 9:28 ` Neil Armstrong
0 siblings, 1 reply; 12+ messages in thread
From: Adrián Larumbe @ 2022-09-20 22:28 UTC (permalink / raw)
To: neil.armstrong; +Cc: khilman, linux-amlogic, dri-devel, adrian.larumbe
drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
were not manually removed at module unload time, which caused dangling
references to freed memory to remain linked in the global bridge_list.
When loading the driver modules back in, the same functions would again
call drm_bridge_add, and when traversing the global bridge_list, would
end up peeking into freed memory.
Once again KASAN revealed the problem:
[ +0.000095] =============================================================
[ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
[ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
[ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
[ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
[ +0.000008] Call trace:
[ +0.000006] dump_backtrace+0x1ec/0x280
[ +0.000012] show_stack+0x24/0x80
[ +0.000008] dump_stack_lvl+0x98/0xd4
[ +0.000011] print_address_description.constprop.0+0x80/0x520
[ +0.000011] print_report+0x128/0x260
[ +0.000008] kasan_report+0xb8/0xfc
[ +0.000008] __asan_report_load8_noabort+0x3c/0x50
[ +0.000009] __list_add_valid+0x9c/0x120
[ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
[ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
[ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
[ +0.000014] component_bind+0x174/0x520
[ +0.000012] component_bind_all+0x1a8/0x38c
[ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
[ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
[ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
[ +0.000010] component_master_add_with_match+0x1c8/0x284
[ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
[ +0.000026] platform_probe+0xd0/0x220
[ +0.000009] really_probe+0x3ac/0xa80
[ +0.000009] __driver_probe_device+0x1f8/0x400
[ +0.000009] driver_probe_device+0x68/0x1b0
[ +0.000009] __driver_attach+0x20c/0x480
[ +0.000008] bus_for_each_dev+0x114/0x1b0
[ +0.000009] driver_attach+0x48/0x64
[ +0.000008] bus_add_driver+0x390/0x564
[ +0.000009] driver_register+0x1a8/0x3e4
[ +0.000009] __platform_driver_register+0x6c/0x94
[ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
[ +0.000027] do_one_initcall+0xc4/0x2b0
[ +0.000011] do_init_module+0x154/0x570
[ +0.000011] load_module+0x1a78/0x1ea4
[ +0.000008] __do_sys_init_module+0x184/0x1cc
[ +0.000009] __arm64_sys_init_module+0x78/0xb0
[ +0.000009] invoke_syscall+0x74/0x260
[ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000012] el0t_64_sync_handler+0x11c/0x150
[ +0.000008] el0t_64_sync+0x18c/0x190
[ +0.000016] Allocated by task 879:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000011] __kasan_kmalloc+0x90/0xd0
[ +0.000007] __kmalloc+0x278/0x4a0
[ +0.000011] mpi_resize+0x13c/0x1d0
[ +0.000011] mpi_powm+0xd24/0x1570
[ +0.000009] rsa_enc+0x1a4/0x30c
[ +0.000009] pkcs1pad_verify+0x3f0/0x580
[ +0.000009] public_key_verify_signature+0x7a8/0xba4
[ +0.000010] public_key_verify_signature_2+0x40/0x60
[ +0.000008] verify_signature+0xb4/0x114
[ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
[ +0.000009] pkcs7_validate_trust+0xb8/0x15c
[ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
[ +0.000012] verify_pkcs7_signature+0x78/0xac
[ +0.000007] mod_verify_sig+0x110/0x190
[ +0.000009] module_sig_check+0x114/0x1e0
[ +0.000009] load_module+0xa0/0x1ea4
[ +0.000008] __do_sys_init_module+0x184/0x1cc
[ +0.000008] __arm64_sys_init_module+0x78/0xb0
[ +0.000008] invoke_syscall+0x74/0x260
[ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000009] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000013] Freed by task 2422:
[ +0.000008] kasan_save_stack+0x2c/0x5c
[ +0.000009] kasan_set_track+0x2c/0x40
[ +0.000007] kasan_set_free_info+0x28/0x50
[ +0.000009] ____kasan_slab_free+0x128/0x1d4
[ +0.000008] __kasan_slab_free+0x18/0x24
[ +0.000007] slab_free_freelist_hook+0x108/0x230
[ +0.000010] kfree+0x110/0x35c
[ +0.000008] release_nodes+0xf0/0x16c
[ +0.000009] devres_release_group+0x180/0x270
[ +0.000008] take_down_aggregate_device+0xcc/0x160
[ +0.000010] component_del+0x18c/0x360
[ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
[ +0.000013] platform_remove+0x64/0xb0
[ +0.000008] device_remove+0xb8/0x154
[ +0.000009] device_release_driver_internal+0x398/0x5b0
[ +0.000009] driver_detach+0xac/0x1b0
[ +0.000009] bus_remove_driver+0x158/0x29c
[ +0.000008] driver_unregister+0x70/0xb0
[ +0.000009] platform_driver_unregister+0x20/0x2c
[ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
[ +0.000012] __do_sys_delete_module+0x288/0x400
[ +0.000009] __arm64_sys_delete_module+0x5c/0x80
[ +0.000009] invoke_syscall+0x74/0x260
[ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
[ +0.000008] do_el0_svc+0x50/0x70
[ +0.000007] el0_svc+0x68/0x1a0
[ +0.000008] el0t_64_sync_handler+0x11c/0x150
[ +0.000009] el0t_64_sync+0x18c/0x190
[ +0.000013] The buggy address belongs to the object at ffff00003da29000
which belongs to the cache kmalloc-1k of size 1024
[ +0.000008] The buggy address is located 496 bytes inside of
1024-byte region [ffff00003da29000, ffff00003da29400)
[ +0.000015] The buggy address belongs to the physical page:
[ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
[ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
[ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
[ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
[ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
[ +0.000008] page dumped because: kasan: bad access detected
[ +0.000011] Memory state around the buggy address:
[ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ^
[ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ==================================================================
Fix by keeping track of which encoders were initialised in the meson_drm
structure and manually removing their bridges at aggregate driver's unbind
time.
Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
---
drivers/gpu/drm/meson/meson_drv.c | 4 ++++
drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
drivers/gpu/drm/meson/meson_encoder_cvbs.c | 13 +++++++++++++
drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
drivers/gpu/drm/meson/meson_encoder_hdmi.c | 13 +++++++++++++
drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
6 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 7649df362a85..3b24a924b7b9 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -390,6 +390,10 @@ static void meson_drv_unbind(struct device *dev)
drm_atomic_helper_shutdown(drm);
free_irq(priv->vsync_irq, drm);
drm_dev_put(drm);
+
+ meson_encoder_hdmi_remove(priv);
+ meson_encoder_cvbs_remove(priv);
+
component_unbind_all(dev, drm);
if (priv->afbcd.ops)
diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
index 177dac3ca3be..c62ee358456f 100644
--- a/drivers/gpu/drm/meson/meson_drv.h
+++ b/drivers/gpu/drm/meson/meson_drv.h
@@ -25,6 +25,12 @@ enum vpu_compatible {
VPU_COMPATIBLE_G12A = 3,
};
+enum {
+ MESON_ENC_CVBS = 0,
+ MESON_ENC_HDMI,
+ MESON_ENC_LAST,
+};
+
struct meson_drm_match_data {
enum vpu_compatible compat;
struct meson_afbcd_ops *afbcd_ops;
@@ -51,6 +57,7 @@ struct meson_drm {
struct drm_crtc *crtc;
struct drm_plane *primary_plane;
struct drm_plane *overlay_plane;
+ void *encoders[MESON_ENC_LAST];
const struct meson_drm_soc_limits *limits;
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 8110a6e39320..5675bc2a92cf 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -281,5 +281,18 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
}
drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
+ priv->encoders[MESON_ENC_CVBS] = meson_encoder_cvbs;
+
return 0;
}
+
+void meson_encoder_cvbs_remove(struct meson_drm *priv)
+{
+ struct meson_encoder_cvbs *meson_encoder_cvbs;
+
+ if (priv->encoders[MESON_ENC_CVBS]) {
+ meson_encoder_cvbs = priv->encoders[MESON_ENC_CVBS];
+ drm_bridge_remove(&meson_encoder_cvbs->bridge);
+ drm_bridge_remove(meson_encoder_cvbs->next_bridge);
+ }
+}
diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
index 61d9d183ce7f..09710fec3c66 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
@@ -25,5 +25,6 @@ struct meson_cvbs_mode {
extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
int meson_encoder_cvbs_init(struct meson_drm *priv);
+void meson_encoder_cvbs_remove(struct meson_drm *priv);
#endif /* __MESON_VENC_CVBS_H */
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 2f616c55c271..53231bfdf7e2 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
meson_encoder_hdmi->cec_notifier = notifier;
}
+ priv->encoders[MESON_ENC_HDMI] = meson_encoder_hdmi;
+
dev_dbg(priv->dev, "HDMI encoder initialized\n");
return 0;
@@ -460,3 +462,14 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
of_node_put(remote);
return ret;
}
+
+void meson_encoder_hdmi_remove(struct meson_drm *priv)
+{
+ struct meson_encoder_hdmi *meson_encoder_hdmi;
+
+ if (priv->encoders[MESON_ENC_HDMI]) {
+ meson_encoder_hdmi = priv->encoders[MESON_ENC_HDMI];
+ drm_bridge_remove(&meson_encoder_hdmi->bridge);
+ drm_bridge_remove(meson_encoder_hdmi->next_bridge);
+ }
+}
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
index ed19494f0956..a6cd38eb5f71 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
@@ -8,5 +8,6 @@
#define __MESON_ENCODER_HDMI_H
int meson_encoder_hdmi_init(struct meson_drm *priv);
+void meson_encoder_hdmi_remove(struct meson_drm *priv);
#endif /* __MESON_ENCODER_HDMI_H */
--
2.37.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/3] drm/meson: remove drm bridges at aggregate driver unbind time
2022-09-20 22:28 ` [PATCH v2 " Adrián Larumbe
@ 2022-09-23 9:28 ` Neil Armstrong
0 siblings, 0 replies; 12+ messages in thread
From: Neil Armstrong @ 2022-09-23 9:28 UTC (permalink / raw)
To: Adrián Larumbe; +Cc: khilman, dri-devel, linux-amlogic
On 21/09/2022 00:28, Adrián Larumbe wrote:
> drm bridges added by meson_encoder_hdmi_init and meson_encoder_cvbs_init
> were not manually removed at module unload time, which caused dangling
> references to freed memory to remain linked in the global bridge_list.
>
> When loading the driver modules back in, the same functions would again
> call drm_bridge_add, and when traversing the global bridge_list, would
> end up peeking into freed memory.
>
> Once again KASAN revealed the problem:
>
> [ +0.000095] =============================================================
> [ +0.000008] BUG: KASAN: use-after-free in __list_add_valid+0x9c/0x120
> [ +0.000018] Read of size 8 at addr ffff00003da291f0 by task modprobe/2483
>
> [ +0.000018] CPU: 3 PID: 2483 Comm: modprobe Tainted: G C O 5.19.0-rc6-lrmbkasan+ #1
> [ +0.000011] Hardware name: Hardkernel ODROID-N2Plus (DT)
> [ +0.000008] Call trace:
> [ +0.000006] dump_backtrace+0x1ec/0x280
> [ +0.000012] show_stack+0x24/0x80
> [ +0.000008] dump_stack_lvl+0x98/0xd4
> [ +0.000011] print_address_description.constprop.0+0x80/0x520
> [ +0.000011] print_report+0x128/0x260
> [ +0.000008] kasan_report+0xb8/0xfc
> [ +0.000008] __asan_report_load8_noabort+0x3c/0x50
> [ +0.000009] __list_add_valid+0x9c/0x120
> [ +0.000009] drm_bridge_add+0x6c/0x104 [drm]
> [ +0.000165] dw_hdmi_probe+0x1900/0x2360 [dw_hdmi]
> [ +0.000022] meson_dw_hdmi_bind+0x520/0x814 [meson_dw_hdmi]
> [ +0.000014] component_bind+0x174/0x520
> [ +0.000012] component_bind_all+0x1a8/0x38c
> [ +0.000010] meson_drv_bind_master+0x5e8/0xb74 [meson_drm]
> [ +0.000032] meson_drv_bind+0x20/0x2c [meson_drm]
> [ +0.000027] try_to_bring_up_aggregate_device+0x19c/0x390
> [ +0.000010] component_master_add_with_match+0x1c8/0x284
> [ +0.000009] meson_drv_probe+0x274/0x280 [meson_drm]
> [ +0.000026] platform_probe+0xd0/0x220
> [ +0.000009] really_probe+0x3ac/0xa80
> [ +0.000009] __driver_probe_device+0x1f8/0x400
> [ +0.000009] driver_probe_device+0x68/0x1b0
> [ +0.000009] __driver_attach+0x20c/0x480
> [ +0.000008] bus_for_each_dev+0x114/0x1b0
> [ +0.000009] driver_attach+0x48/0x64
> [ +0.000008] bus_add_driver+0x390/0x564
> [ +0.000009] driver_register+0x1a8/0x3e4
> [ +0.000009] __platform_driver_register+0x6c/0x94
> [ +0.000008] meson_drm_platform_driver_init+0x3c/0x1000 [meson_drm]
> [ +0.000027] do_one_initcall+0xc4/0x2b0
> [ +0.000011] do_init_module+0x154/0x570
> [ +0.000011] load_module+0x1a78/0x1ea4
> [ +0.000008] __do_sys_init_module+0x184/0x1cc
> [ +0.000009] __arm64_sys_init_module+0x78/0xb0
> [ +0.000009] invoke_syscall+0x74/0x260
> [ +0.000009] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000012] el0t_64_sync_handler+0x11c/0x150
> [ +0.000008] el0t_64_sync+0x18c/0x190
>
> [ +0.000016] Allocated by task 879:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000011] __kasan_kmalloc+0x90/0xd0
> [ +0.000007] __kmalloc+0x278/0x4a0
> [ +0.000011] mpi_resize+0x13c/0x1d0
> [ +0.000011] mpi_powm+0xd24/0x1570
> [ +0.000009] rsa_enc+0x1a4/0x30c
> [ +0.000009] pkcs1pad_verify+0x3f0/0x580
> [ +0.000009] public_key_verify_signature+0x7a8/0xba4
> [ +0.000010] public_key_verify_signature_2+0x40/0x60
> [ +0.000008] verify_signature+0xb4/0x114
> [ +0.000008] pkcs7_validate_trust_one.constprop.0+0x3b8/0x574
> [ +0.000009] pkcs7_validate_trust+0xb8/0x15c
> [ +0.000008] verify_pkcs7_message_sig+0xec/0x1b0
> [ +0.000012] verify_pkcs7_signature+0x78/0xac
> [ +0.000007] mod_verify_sig+0x110/0x190
> [ +0.000009] module_sig_check+0x114/0x1e0
> [ +0.000009] load_module+0xa0/0x1ea4
> [ +0.000008] __do_sys_init_module+0x184/0x1cc
> [ +0.000008] __arm64_sys_init_module+0x78/0xb0
> [ +0.000008] invoke_syscall+0x74/0x260
> [ +0.000009] el0_svc_common.constprop.0+0x1a8/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000009] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] Freed by task 2422:
> [ +0.000008] kasan_save_stack+0x2c/0x5c
> [ +0.000009] kasan_set_track+0x2c/0x40
> [ +0.000007] kasan_set_free_info+0x28/0x50
> [ +0.000009] ____kasan_slab_free+0x128/0x1d4
> [ +0.000008] __kasan_slab_free+0x18/0x24
> [ +0.000007] slab_free_freelist_hook+0x108/0x230
> [ +0.000010] kfree+0x110/0x35c
> [ +0.000008] release_nodes+0xf0/0x16c
> [ +0.000009] devres_release_group+0x180/0x270
> [ +0.000008] take_down_aggregate_device+0xcc/0x160
> [ +0.000010] component_del+0x18c/0x360
> [ +0.000009] meson_dw_hdmi_remove+0x28/0x40 [meson_dw_hdmi]
> [ +0.000013] platform_remove+0x64/0xb0
> [ +0.000008] device_remove+0xb8/0x154
> [ +0.000009] device_release_driver_internal+0x398/0x5b0
> [ +0.000009] driver_detach+0xac/0x1b0
> [ +0.000009] bus_remove_driver+0x158/0x29c
> [ +0.000008] driver_unregister+0x70/0xb0
> [ +0.000009] platform_driver_unregister+0x20/0x2c
> [ +0.000007] meson_dw_hdmi_platform_driver_exit+0x1c/0x30 [meson_dw_hdmi]
> [ +0.000012] __do_sys_delete_module+0x288/0x400
> [ +0.000009] __arm64_sys_delete_module+0x5c/0x80
> [ +0.000009] invoke_syscall+0x74/0x260
> [ +0.000008] el0_svc_common.constprop.0+0xcc/0x260
> [ +0.000008] do_el0_svc+0x50/0x70
> [ +0.000007] el0_svc+0x68/0x1a0
> [ +0.000008] el0t_64_sync_handler+0x11c/0x150
> [ +0.000009] el0t_64_sync+0x18c/0x190
>
> [ +0.000013] The buggy address belongs to the object at ffff00003da29000
> which belongs to the cache kmalloc-1k of size 1024
> [ +0.000008] The buggy address is located 496 bytes inside of
> 1024-byte region [ffff00003da29000, ffff00003da29400)
>
> [ +0.000015] The buggy address belongs to the physical page:
> [ +0.000009] page:fffffc0000f68a00 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x3da28
> [ +0.000012] head:fffffc0000f68a00 order:3 compound_mapcount:0 compound_pincount:0
> [ +0.000009] flags: 0xffff00000010200(slab|head|node=0|zone=0|lastcpupid=0xffff)
> [ +0.000019] raw: 0ffff00000010200 fffffc0000eb5c08 fffffc0000d96608 ffff000000002a80
> [ +0.000008] raw: 0000000000000000 00000000000a000a 00000001ffffffff 0000000000000000
> [ +0.000008] page dumped because: kasan: bad access detected
>
> [ +0.000011] Memory state around the buggy address:
> [ +0.000009] ffff00003da29080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ffff00003da29100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] >ffff00003da29180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ^
> [ +0.000008] ffff00003da29200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000006] ffff00003da29280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> [ +0.000007] ==================================================================
>
> Fix by keeping track of which encoders were initialised in the meson_drm
> structure and manually removing their bridges at aggregate driver's unbind
> time.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 4 ++++
> drivers/gpu/drm/meson/meson_drv.h | 7 +++++++
> drivers/gpu/drm/meson/meson_encoder_cvbs.c | 13 +++++++++++++
> drivers/gpu/drm/meson/meson_encoder_cvbs.h | 1 +
> drivers/gpu/drm/meson/meson_encoder_hdmi.c | 13 +++++++++++++
> drivers/gpu/drm/meson/meson_encoder_hdmi.h | 1 +
> 6 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 7649df362a85..3b24a924b7b9 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -390,6 +390,10 @@ static void meson_drv_unbind(struct device *dev)
> drm_atomic_helper_shutdown(drm);
> free_irq(priv->vsync_irq, drm);
> drm_dev_put(drm);
> +
> + meson_encoder_hdmi_remove(priv);
> + meson_encoder_cvbs_remove(priv);
> +
> component_unbind_all(dev, drm);
>
> if (priv->afbcd.ops)
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 177dac3ca3be..c62ee358456f 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -25,6 +25,12 @@ enum vpu_compatible {
> VPU_COMPATIBLE_G12A = 3,
> };
>
> +enum {
> + MESON_ENC_CVBS = 0,
> + MESON_ENC_HDMI,
> + MESON_ENC_LAST,
> +};
> +
> struct meson_drm_match_data {
> enum vpu_compatible compat;
> struct meson_afbcd_ops *afbcd_ops;
> @@ -51,6 +57,7 @@ struct meson_drm {
> struct drm_crtc *crtc;
> struct drm_plane *primary_plane;
> struct drm_plane *overlay_plane;
> + void *encoders[MESON_ENC_LAST];
>
> const struct meson_drm_soc_limits *limits;
>
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> index 8110a6e39320..5675bc2a92cf 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
> @@ -281,5 +281,18 @@ int meson_encoder_cvbs_init(struct meson_drm *priv)
> }
> drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
>
> + priv->encoders[MESON_ENC_CVBS] = meson_encoder_cvbs;
> +
> return 0;
> }
> +
> +void meson_encoder_cvbs_remove(struct meson_drm *priv)
> +{
> + struct meson_encoder_cvbs *meson_encoder_cvbs;
> +
> + if (priv->encoders[MESON_ENC_CVBS]) {
> + meson_encoder_cvbs = priv->encoders[MESON_ENC_CVBS];
> + drm_bridge_remove(&meson_encoder_cvbs->bridge);
> + drm_bridge_remove(meson_encoder_cvbs->next_bridge);
> + }
> +}
> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> index 61d9d183ce7f..09710fec3c66 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
> @@ -25,5 +25,6 @@ struct meson_cvbs_mode {
> extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
>
> int meson_encoder_cvbs_init(struct meson_drm *priv);
> +void meson_encoder_cvbs_remove(struct meson_drm *priv);
>
> #endif /* __MESON_VENC_CVBS_H */
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> index 2f616c55c271..53231bfdf7e2 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -452,6 +452,8 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> meson_encoder_hdmi->cec_notifier = notifier;
> }
>
> + priv->encoders[MESON_ENC_HDMI] = meson_encoder_hdmi;
> +
> dev_dbg(priv->dev, "HDMI encoder initialized\n");
>
> return 0;
> @@ -460,3 +462,14 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
> of_node_put(remote);
> return ret;
> }
> +
> +void meson_encoder_hdmi_remove(struct meson_drm *priv)
> +{
> + struct meson_encoder_hdmi *meson_encoder_hdmi;
> +
> + if (priv->encoders[MESON_ENC_HDMI]) {
> + meson_encoder_hdmi = priv->encoders[MESON_ENC_HDMI];
> + drm_bridge_remove(&meson_encoder_hdmi->bridge);
> + drm_bridge_remove(meson_encoder_hdmi->next_bridge);
> + }
> +}
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> index ed19494f0956..a6cd38eb5f71 100644
> --- a/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> @@ -8,5 +8,6 @@
> #define __MESON_ENCODER_HDMI_H
>
> int meson_encoder_hdmi_init(struct meson_drm *priv);
> +void meson_encoder_hdmi_remove(struct meson_drm *priv);
>
> #endif /* __MESON_ENCODER_HDMI_H */
Great, thanks !
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: (subset) [PATCH 0/3] drm/meson: fix use-after-free driver unload issues
2022-09-19 1:09 [PATCH 0/3] drm/meson: fix use-after-free driver unload issues Adrián Larumbe
` (2 preceding siblings ...)
2022-09-19 1:09 ` [PATCH 3/3] drm/meson: remove drm bridges at aggregate driver unbind time Adrián Larumbe
@ 2022-09-23 9:48 ` Neil Armstrong
3 siblings, 0 replies; 12+ messages in thread
From: Neil Armstrong @ 2022-09-23 9:48 UTC (permalink / raw)
To: narmstrong, linux-amlogic, dri-devel, Adrián Larumbe, khilman
Cc: Neil Armstrong
Hi,
On Mon, 19 Sep 2022 02:09:37 +0100, Adrián Larumbe wrote:
> This patch series tries to fix some use-after-free bugs I've observed with
> the help of KASAN in Amlogic's KMS DRM driver.
>
> The first patch in the series reorders the driver deinitialisation sequence
> so that devres won't deallocate things that are still expected to be around
> by a later call to drm_dev_put.
>
> [...]
Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
[1/3] drm/meson: reorder driver deinit sequence to fix use-after-free bug
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=31c519981eb141c7ec39bfd5be25d35f02edb868
[2/3] drm/meson: explicitly remove aggregate driver at module unload time
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8616f2a0589a80e08434212324250eb22f6a66ce
[3/3] drm/meson: remove drm bridges at aggregate driver unbind time
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=09847723c12fc2753749cec3939a02ee92dac468
--
Neil
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 12+ messages in thread