| From 1f6129253f70147115bcd1d4864f31b0b9f4ed22 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 6 May 2022 00:05:40 +0200 |
| Subject: fbdev: efifb: Cleanup fb_info in .fb_destroy rather than .remove |
| |
| From: Javier Martinez Canillas <javierm@redhat.com> |
| |
| [ Upstream commit d258d00fb9c7c0cdf9d10c1ded84f10339d2d349 ] |
| |
| The driver is calling framebuffer_release() in its .remove callback, but |
| this will cause the struct fb_info to be freed too early. Since it could |
| be that a reference is still hold to it if user-space opened the fbdev. |
| |
| This would lead to a use-after-free error if the framebuffer device was |
| unregistered but later a user-space process tries to close the fbdev fd. |
| |
| To prevent this, move the framebuffer_release() call to fb_ops.fb_destroy |
| instead of doing it in the driver's .remove callback. |
| |
| Strictly speaking, the code flow in the driver is still wrong because all |
| the hardware cleanupd (i.e: iounmap) should be done in .remove while the |
| software cleanup (i.e: releasing the framebuffer) should be done in the |
| .fb_destroy handler. But this at least makes to match the behavior before |
| commit 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal"). |
| |
| Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal") |
| Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> |
| Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> |
| Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de> |
| Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> |
| Link: https://patchwork.freedesktop.org/patch/msgid/20220505220540.366218-1-javierm@redhat.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/video/fbdev/efifb.c | 9 ++++++++- |
| 1 file changed, 8 insertions(+), 1 deletion(-) |
| |
| diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c |
| index ea42ba6445b2..cfa3dc0b4eee 100644 |
| --- a/drivers/video/fbdev/efifb.c |
| +++ b/drivers/video/fbdev/efifb.c |
| @@ -243,6 +243,10 @@ static void efifb_show_boot_graphics(struct fb_info *info) |
| static inline void efifb_show_boot_graphics(struct fb_info *info) {} |
| #endif |
| |
| +/* |
| + * fb_ops.fb_destroy is called by the last put_fb_info() call at the end |
| + * of unregister_framebuffer() or fb_release(). Do any cleanup here. |
| + */ |
| static void efifb_destroy(struct fb_info *info) |
| { |
| if (efifb_pci_dev) |
| @@ -254,6 +258,9 @@ static void efifb_destroy(struct fb_info *info) |
| else |
| memunmap(info->screen_base); |
| } |
| + |
| + framebuffer_release(info); |
| + |
| if (request_mem_succeeded) |
| release_mem_region(info->apertures->ranges[0].base, |
| info->apertures->ranges[0].size); |
| @@ -620,9 +627,9 @@ static int efifb_remove(struct platform_device *pdev) |
| { |
| struct fb_info *info = platform_get_drvdata(pdev); |
| |
| + /* efifb_destroy takes care of info cleanup */ |
| unregister_framebuffer(info); |
| sysfs_remove_groups(&pdev->dev.kobj, efifb_groups); |
| - framebuffer_release(info); |
| |
| return 0; |
| } |
| -- |
| 2.35.1 |
| |