diff mbox series

[V2] drm/mediatek: Set private->all_drm_private[i]->drm to NULL if mtk_drm_bind returns err

Message ID 20241223023227.1258112-1-guoqing.jiang@canonical.com (mailing list archive)
State New
Headers show
Series [V2] drm/mediatek: Set private->all_drm_private[i]->drm to NULL if mtk_drm_bind returns err | expand

Commit Message

Guoqing Jiang Dec. 23, 2024, 2:32 a.m. UTC
The pointer need to be set to NULL, otherwise KASAN complains about
use-after-free. Because in mtk_drm_bind, all private's drm are set
as follows.

private->all_drm_private[i]->drm = drm;

And drm will be released by drm_dev_put in case mtk_drm_kms_init returns
failure. However, the shutdown path still accesses the previous allocated
memory in drm_atomic_helper_shutdown.

[   84.874820] watchdog: watchdog0: watchdog did not stop!
[   86.512054] ==================================================================
[   86.513162] BUG: KASAN: use-after-free in drm_atomic_helper_shutdown+0x33c/0x378
[   86.514258] Read of size 8 at addr ffff0000d46fc068 by task shutdown/1
[   86.515213]
[   86.515455] CPU: 1 UID: 0 PID: 1 Comm: shutdown Not tainted 6.13.0-rc1-mtk+gfa1a78e5d24b-dirty #55
[   86.516752] Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2022.10 10/01/2022
[   86.517960] Call trace:
[   86.518333]  show_stack+0x20/0x38 (C)
[   86.518891]  dump_stack_lvl+0x90/0xd0
[   86.519443]  print_report+0xf8/0x5b0
[   86.519985]  kasan_report+0xb4/0x100
[   86.520526]  __asan_report_load8_noabort+0x20/0x30
[   86.521240]  drm_atomic_helper_shutdown+0x33c/0x378
[   86.521966]  mtk_drm_shutdown+0x54/0x80
[   86.522546]  platform_shutdown+0x64/0x90
[   86.523137]  device_shutdown+0x260/0x5b8
[   86.523728]  kernel_restart+0x78/0xf0
[   86.524282]  __do_sys_reboot+0x258/0x2f0
[   86.524871]  __arm64_sys_reboot+0x90/0xd8
[   86.525473]  invoke_syscall+0x74/0x268
[   86.526041]  el0_svc_common.constprop.0+0xb0/0x240
[   86.526751]  do_el0_svc+0x4c/0x70
[   86.527251]  el0_svc+0x4c/0xc0
[   86.527719]  el0t_64_sync_handler+0x144/0x168
[   86.528367]  el0t_64_sync+0x198/0x1a0
[   86.528920]
[   86.529157] The buggy address belongs to the physical page:
[   86.529972] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff0000d46fd4d0 pfn:0x1146fc
[   86.531319] flags: 0xbfffc0000000000(node=0|zone=2|lastcpupid=0xffff)
[   86.532267] raw: 0bfffc0000000000 0000000000000000 dead000000000122 0000000000000000
[   86.533390] raw: ffff0000d46fd4d0 0000000000000000 00000000ffffffff 0000000000000000
[   86.534511] page dumped because: kasan: bad access detected
[   86.535323]
[   86.535559] Memory state around the buggy address:
[   86.536265]  ffff0000d46fbf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   86.537314]  ffff0000d46fbf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   86.538363] >ffff0000d46fc000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   86.544733]                                                           ^
[   86.551057]  ffff0000d46fc080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   86.557510]  ffff0000d46fc100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   86.563928] ==================================================================
[   86.571093] Disabling lock debugging due to kernel taint
[   86.577642] Unable to handle kernel paging request at virtual address e0e9c0920000000b
[   86.581834] KASAN: maybe wild-memory-access in range [0x0752049000000058-0x075204900000005f]
...

Fixes: 1ef7ed48356c ("drm/mediatek: Modify mediatek-drm for mt8195 multi mmsys support")
Signed-off-by: Guoqing Jiang <guoqing.jiang@canonical.com>
---
V2 changes:
1. add Fixes tag per CK's comment

 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

CK Hu (胡俊光) Dec. 23, 2024, 2:56 a.m. UTC | #1
Hi, Guoqing:

On Mon, 2024-12-23 at 10:32 +0800, Guoqing Jiang wrote:
> External email : Please do not click links or open attachments until you have verified the sender or the content.
> 
> 
> The pointer need to be set to NULL, otherwise KASAN complains about
> use-after-free. Because in mtk_drm_bind, all private's drm are set
> as follows.
> 
> private->all_drm_private[i]->drm = drm;
> 
> And drm will be released by drm_dev_put in case mtk_drm_kms_init returns
> failure. However, the shutdown path still accesses the previous allocated
> memory in drm_atomic_helper_shutdown.
> 
> [   84.874820] watchdog: watchdog0: watchdog did not stop!
> [   86.512054] ==================================================================
> [   86.513162] BUG: KASAN: use-after-free in drm_atomic_helper_shutdown+0x33c/0x378
> [   86.514258] Read of size 8 at addr ffff0000d46fc068 by task shutdown/1
> [   86.515213]
> [   86.515455] CPU: 1 UID: 0 PID: 1 Comm: shutdown Not tainted 6.13.0-rc1-mtk+gfa1a78e5d24b-dirty #55
> [   86.516752] Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2022.10 10/01/2022
> [   86.517960] Call trace:
> [   86.518333]  show_stack+0x20/0x38 (C)
> [   86.518891]  dump_stack_lvl+0x90/0xd0
> [   86.519443]  print_report+0xf8/0x5b0
> [   86.519985]  kasan_report+0xb4/0x100
> [   86.520526]  __asan_report_load8_noabort+0x20/0x30
> [   86.521240]  drm_atomic_helper_shutdown+0x33c/0x378
> [   86.521966]  mtk_drm_shutdown+0x54/0x80
> [   86.522546]  platform_shutdown+0x64/0x90
> [   86.523137]  device_shutdown+0x260/0x5b8
> [   86.523728]  kernel_restart+0x78/0xf0
> [   86.524282]  __do_sys_reboot+0x258/0x2f0
> [   86.524871]  __arm64_sys_reboot+0x90/0xd8
> [   86.525473]  invoke_syscall+0x74/0x268
> [   86.526041]  el0_svc_common.constprop.0+0xb0/0x240
> [   86.526751]  do_el0_svc+0x4c/0x70
> [   86.527251]  el0_svc+0x4c/0xc0
> [   86.527719]  el0t_64_sync_handler+0x144/0x168
> [   86.528367]  el0t_64_sync+0x198/0x1a0
> [   86.528920]
> [   86.529157] The buggy address belongs to the physical page:
> [   86.529972] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff0000d46fd4d0 pfn:0x1146fc
> [   86.531319] flags: 0xbfffc0000000000(node=0|zone=2|lastcpupid=0xffff)
> [   86.532267] raw: 0bfffc0000000000 0000000000000000 dead000000000122 0000000000000000
> [   86.533390] raw: ffff0000d46fd4d0 0000000000000000 00000000ffffffff 0000000000000000
> [   86.534511] page dumped because: kasan: bad access detected
> [   86.535323]
> [   86.535559] Memory state around the buggy address:
> [   86.536265]  ffff0000d46fbf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.537314]  ffff0000d46fbf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.538363] >ffff0000d46fc000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.544733]                                                           ^
> [   86.551057]  ffff0000d46fc080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.557510]  ffff0000d46fc100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.563928] ==================================================================
> [   86.571093] Disabling lock debugging due to kernel taint
> [   86.577642] Unable to handle kernel paging request at virtual address e0e9c0920000000b
> [   86.581834] KASAN: maybe wild-memory-access in range [0x0752049000000058-0x075204900000005f]
> ...

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fixes: 1ef7ed48356c ("drm/mediatek: Modify mediatek-drm for mt8195 multi mmsys support")
> Signed-off-by: Guoqing Jiang <guoqing.jiang@canonical.com>
> ---
> V2 changes:
> 1. add Fixes tag per CK's comment
> 
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index 9a8ef8558da9..0062374f75d5 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -673,6 +673,8 @@ static int mtk_drm_bind(struct device *dev)
>  err_free:
>         private->drm = NULL;
>         drm_dev_put(drm);
> +       for (i = 0; i < private->data->mmsys_dev_num; i++)
> +               private->all_drm_private[i]->drm = NULL;
>         return ret;
>  }
> 
> --
> 2.35.3
> 
>
AngeloGioacchino Del Regno Dec. 23, 2024, 11:23 a.m. UTC | #2
Il 23/12/24 03:32, Guoqing Jiang ha scritto:
> The pointer need to be set to NULL, otherwise KASAN complains about
> use-after-free. Because in mtk_drm_bind, all private's drm are set
> as follows.
> 
> private->all_drm_private[i]->drm = drm;
> 
> And drm will be released by drm_dev_put in case mtk_drm_kms_init returns
> failure. However, the shutdown path still accesses the previous allocated
> memory in drm_atomic_helper_shutdown.
> 
> [   84.874820] watchdog: watchdog0: watchdog did not stop!
> [   86.512054] ==================================================================
> [   86.513162] BUG: KASAN: use-after-free in drm_atomic_helper_shutdown+0x33c/0x378
> [   86.514258] Read of size 8 at addr ffff0000d46fc068 by task shutdown/1
> [   86.515213]
> [   86.515455] CPU: 1 UID: 0 PID: 1 Comm: shutdown Not tainted 6.13.0-rc1-mtk+gfa1a78e5d24b-dirty #55
> [   86.516752] Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2022.10 10/01/2022
> [   86.517960] Call trace:
> [   86.518333]  show_stack+0x20/0x38 (C)
> [   86.518891]  dump_stack_lvl+0x90/0xd0
> [   86.519443]  print_report+0xf8/0x5b0
> [   86.519985]  kasan_report+0xb4/0x100
> [   86.520526]  __asan_report_load8_noabort+0x20/0x30
> [   86.521240]  drm_atomic_helper_shutdown+0x33c/0x378
> [   86.521966]  mtk_drm_shutdown+0x54/0x80
> [   86.522546]  platform_shutdown+0x64/0x90
> [   86.523137]  device_shutdown+0x260/0x5b8
> [   86.523728]  kernel_restart+0x78/0xf0
> [   86.524282]  __do_sys_reboot+0x258/0x2f0
> [   86.524871]  __arm64_sys_reboot+0x90/0xd8
> [   86.525473]  invoke_syscall+0x74/0x268
> [   86.526041]  el0_svc_common.constprop.0+0xb0/0x240
> [   86.526751]  do_el0_svc+0x4c/0x70
> [   86.527251]  el0_svc+0x4c/0xc0
> [   86.527719]  el0t_64_sync_handler+0x144/0x168
> [   86.528367]  el0t_64_sync+0x198/0x1a0
> [   86.528920]
> [   86.529157] The buggy address belongs to the physical page:
> [   86.529972] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff0000d46fd4d0 pfn:0x1146fc
> [   86.531319] flags: 0xbfffc0000000000(node=0|zone=2|lastcpupid=0xffff)
> [   86.532267] raw: 0bfffc0000000000 0000000000000000 dead000000000122 0000000000000000
> [   86.533390] raw: ffff0000d46fd4d0 0000000000000000 00000000ffffffff 0000000000000000
> [   86.534511] page dumped because: kasan: bad access detected
> [   86.535323]
> [   86.535559] Memory state around the buggy address:
> [   86.536265]  ffff0000d46fbf00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.537314]  ffff0000d46fbf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.538363] >ffff0000d46fc000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.544733]                                                           ^
> [   86.551057]  ffff0000d46fc080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.557510]  ffff0000d46fc100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [   86.563928] ==================================================================
> [   86.571093] Disabling lock debugging due to kernel taint
> [   86.577642] Unable to handle kernel paging request at virtual address e0e9c0920000000b
> [   86.581834] KASAN: maybe wild-memory-access in range [0x0752049000000058-0x075204900000005f]
> ...
> 
> Fixes: 1ef7ed48356c ("drm/mediatek: Modify mediatek-drm for mt8195 multi mmsys support")
> Signed-off-by: Guoqing Jiang <guoqing.jiang@canonical.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 9a8ef8558da9..0062374f75d5 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -673,6 +673,8 @@  static int mtk_drm_bind(struct device *dev)
 err_free:
 	private->drm = NULL;
 	drm_dev_put(drm);
+	for (i = 0; i < private->data->mmsys_dev_num; i++)
+		private->all_drm_private[i]->drm = NULL;
 	return ret;
 }