[PATCH] fbdev: smscufx: don't free registered framebuffer on probe error

[YuanZhe Liu]

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@xxxxxxxxxxxxxxx
---
 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