Hi Am 03.05.22 um 14:29 schrieb Chang, Junxiao:
We tested Javier's fix, issue couldn't be reproduced anymore. https://lore.kernel.org/dri-devel/20220502135014.377945-1-javierm@xxxxxxxxxx/T/#u
Thank you so much. Best regards Thomas
But this fix doesn't cover all FB driver's interface: .get_unmapped_area = get_fb_unmapped_area, .fsync = fb_deferred_io_fsync, file_fb_info(file) NULL checking should be added in these two interface functions(get_fb_unmapped_area and fb_deferred_io_fsync) as well? Regards, Junxiao -----Original Message----- From: Thomas Zimmermann <tzimmermann@xxxxxxx> Sent: Tuesday, May 3, 2022 3:16 PM To: Chang, Junxiao <junxiao.chang@xxxxxxxxx>; linux-fbdev@xxxxxxxxxxxxxxx Cc: lethal@xxxxxxxxxxxx; patchwork-bot@xxxxxxxxxx; deller@xxxxxx; Li, Lili <lili.li@xxxxxxxxx>; Javier Martinez Canillas <javierm@xxxxxxxxxx> Subject: Re: [PATCH] video: fbdev: don't remove firmware fb device if it is busy (cc'ing Javier) Hi Am 03.05.22 um 02:38 schrieb Chang, Junxiao:Hi Thomas, We reproduced this issue with Yocto image + 5.18-rc3 kernel. If you could try Yocto image and enable psplash, the problem could be reproduced almost every time if psplash is launched before Intel i915 registers 2nd framebuffer. During Yocto booting up, it launches psplash which opens EFI firmware framebuffer and plays animation. When it exits, fb_release in kernel might access memory which has been released. Overall process looks like: 1. EFI registers framebuffer in efifb_probe, framebuffer_alloc is called to allocate "struct fb_info" memory; 2. In userspace, psplash is started to play boot animation, it opens framebuffer driver. In fb_open function, it saves fb_info memory to file->private_data; 3. When Intel i915 driver is probed, it registers 2nd framebuffer, and will remove conflicting framebuffer; 4. In do_remove_conflicting_framebuffers, it calls "platform_device_unregister" to remove EFI platform framebuffer device; 5. In EFI framebuffer's efifb_remove function, it calls framebuffer_release to release "struct fb_info" memory which is still use and stored in file->private_data; 6. When psplash user space application exits, it calls fb_release function, and accesses to fb_info memory, but it has been released in step 5. Kernel panic happens.Thanks for the information. We only had a similar report about RPi devices, but we never encountered this problem before.Our patch is to check whether EFI framebuffer is opened at that time. If it is free(registered_fb[i]->count == 1), go ahead to remove EFI platform device. Or else, just unregister framebuffer to avoid kernel panic.Javier posted a possible fix for the root cause of this problem, but we're still looking for testers. If you have the opportunity, could you please test the patch at [1] and report back on the results. Best regards Thomas [1] https://lore.kernel.org/dri-devel/20220502135014.377945-1-javierm@xxxxxxxxxx/T/#uThanks, Junxiao -----Original Message----- From: Thomas Zimmermann <tzimmermann@xxxxxxx> Sent: Monday, May 2, 2022 3:24 PM To: Chang, Junxiao <junxiao.chang@xxxxxxxxx>; linux-fbdev@xxxxxxxxxxxxxxx Cc: lethal@xxxxxxxxxxxx; patchwork-bot@xxxxxxxxxx; deller@xxxxxx; Li, Lili <lili.li@xxxxxxxxx> Subject: Re: [PATCH] video: fbdev: don't remove firmware fb device if it is busy Hi Am 30.04.22 um 04:57 schrieb Junxiao Chang:When firmware framebuffer is in use, don't remove its platform device. Or else its fb_info buffer is released by platform remove hook while device is still opened. This introduces use after free issue.When exactly does this happen? Do you have a reliable way of reproducing it? Best regards ThomasA kernel panic example: CPU: 2 PID: 3425 Comm: psplash Tainted: G U W 5.18.0-rc3 Hardware name: Intel Client Platform/ADP-S DDR5 UDIMM CRB RIP: 0010:native_queued_spin_lock_slowpath+0x1c7/0x210 RSP: 0018:ffffb3a0c0c2fdb0 EFLAGS: 00010206 RAX: 002dc074ff5c0988 RBX: ffff92e987a5d818 RCX: ffff92e989ba9f40 RDX: 0000000000002067 RSI: ffffffff864344f1 RDI: ffffffff8644183c RBP: ffff92f10f4abd40 R08: 0000000000000001 R09: ffff92e986dc2188 ... Call Trace: <TASK> _raw_spin_lock+0x2c/0x30 __mutex_lock.constprop.0+0x175/0x4f0 ? _raw_spin_unlock+0x15/0x30 ? list_lru_add+0x124/0x160 fb_release+0x1b/0x60 __fput+0x89/0x240 task_work_run+0x59/0x90 do_exit+0x343/0xaf0 do_group_exit+0x2d/0x90 __x64_sys_exit_group+0x14/0x20 do_syscall_64+0x40/0x90 entry_SYSCALL_64_after_hwframe+0x44/0xae Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal") Signed-off-by: Junxiao Chang <junxiao.chang@xxxxxxxxx> Signed-off-by: Lili Li <lili.li@xxxxxxxxx> --- drivers/video/fbdev/core/fbmem.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index a6bb0e438216..ff9b9830b398 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1586,7 +1586,9 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, * framebuffer as before without warning. */ do_unregister_framebuffer(registered_fb[i]); - } else if (dev_is_platform(device)) { + } else if (dev_is_platform(device) && + refcount_read(®istered_fb[i]->count) == 1) { + /* Remove platform device if it is not in use. */ registered_fb[i]->forced_out = true; platform_device_unregister(to_platform_device(device)); } else {-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
