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: stable@vger.kernel.org --- 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