register_framebuffer() can return an error after the framebuffer has
already been installed: do_register_framebuffer() bumps info->count,
creates /dev/fbN and stores the fb_info in registered_fb[] before it
returns the result of the console takeover, and that takeover may fail.
ufx_usb_probe() treats any error as "nothing was registered" and unwinds
through reset_active, which releases the fb_info and drops both
references to struct ufx_data, freeing @dev.
The framebuffer stays registered and can still be opened, so the next
open() of /dev/fbN reaches ufx_ops_open() with info->par pointing at
freed memory:
refcount_t: addition on 0; use-after-free.
WARNING: lib/refcount.c:25 at refcount_warn_saturate
Call Trace:
kref_get
ufx_ops_open drivers/video/fbdev/smscufx.c:1033
fb_open drivers/video/fbdev/core/fb_chrdev.c:351
chrdev_open
...
refcount_t: underflow; use-after-free.
WARNING: lib/refcount.c:28 at refcount_warn_saturate
Call Trace:
ufx_ops_release drivers/video/fbdev/smscufx.c:1135
fb_release drivers/video/fbdev/core/fb_chrdev.c:378
__fput
Reuse the "has this fb_info been registered" test that ufx_setup_modes()
already uses: take the error path only when refcount_read(&info->count)
says the core did not register the framebuffer. A failed console
takeover is not fatal for the driver, so report it and keep the
framebuffer registered; the disconnect callback will unregister it
normally.
Fixes: 3c8a63e22a08 ("Add support for SMSC UFX6000/7000 USB display adapters")
Cc: [email protected]
---
drivers/video/fbdev/smscufx.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 5704f60e1741..2aa001d58aaa 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -1697,8 +1697,16 @@ static int ufx_usb_probe(struct usb_interface *interface,
dev_dbg(dev->gdev, "registering framebuffer");
retval = register_framebuffer(info);
if (retval < 0) {
- dev_err(dev->gdev, "error %d register_framebuffer", retval);
- goto reset_active;
+ /*
+ * info is installed before the takeover can fail; freeing @dev
+ * here would leave registered_fb[] and /dev/fbN pointing at
+ * freed memory.
+ */
+ if (!refcount_read(&info->count)) { /* not registered */
+ dev_err(dev->gdev, "error %d register_framebuffer",
retval);
+ goto reset_active;
+ }
+ dev_warn(dev->gdev, "console takeover failed: %d\n", retval);
}
dev_info(dev->gdev, "SMSC UDX USB device /dev/fb%d attached. %dx%d
resolution."
--
2.34.1
