Hi,
On 1/20/20 11:00 AM, Gerd Hoffmann wrote:
Problem: do_unregister_framebuffer() might return before the device is fully cleaned up, due to userspace having a file handle for /dev/fb0
do_unregister_framebuffer() doesn't guarantee that fb_info is freed after function's return (it only drops the kernel reference on fb_info).
open. Which can result in drm driver not being able to grab resources (and fail initialization) because the firmware framebuffer still holds them. Reportedly plymouth can trigger this.
Could you please describe issue some more?
I guess that a problem is happening during DRM driver load while fbdev driver is loaded? I assume do_unregister_framebuffer() is called inside do_remove_conflicting_framebuffers()?
At first glance it seems to be an user-space issue as it should not be holding references on /dev/fb0 while DRM driver is being loaded.
Fix this by trying to wait until all references are gone. Don't wait forever though given that userspace might keep the file handle open.
Where does the 1s maximum delay come from?
Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Reported-by: Marek Marczykowski-Górecki marmarek@invisiblethingslab.com Signed-off-by: Gerd Hoffmann kraxel@redhat.com
drivers/video/fbdev/core/fbmem.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index d04554959ea7..2ea8ac05b065 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -35,6 +35,7 @@ #include <linux/fbcon.h> #include <linux/mem_encrypt.h> #include <linux/pci.h> +#include <linux/delay.h>
#include <asm/fb.h>
@@ -1707,6 +1708,8 @@ static void unlink_framebuffer(struct fb_info *fb_info)
static void do_unregister_framebuffer(struct fb_info *fb_info) {
- int limit = 100;
- unlink_framebuffer(fb_info); if (fb_info->pixmap.addr && (fb_info->pixmap.flags & FB_PIXMAP_DEFAULT))
@@ -1726,6 +1729,10 @@ static void do_unregister_framebuffer(struct fb_info *fb_info) fbcon_fb_unregistered(fb_info); console_unlock();
- /* try wait until all references are gone */
- while (atomic_read(&fb_info->count) > 1 && --limit > 0)
msleep(10);
- /* this may free fb info */ put_fb_info(fb_info);
}