RE: [PATCH] video: fbdev: don't remove firmware fb device if it is busy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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. 

Thanks,
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
Thomas

> 
> A 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(&registered_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




[Index of Archives]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Tourism]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux