[PATCH v2] fbdev: smscufx: keep the usb_device alive while ufx_data is in use

[YuanZhe Liu]

ufx_usb_probe() stores the usb_device pointer in dev->udev without
taking a reference, and ufx_usb_disconnect() does not wait for in-flight
control transfers.  A mode set or blank request that runs while the
device is being unplugged - through ufx_ops_set_par() or ufx_ops_blank()
on an open /dev/fbN file - keeps issuing synchronous control messages for
the whole ufx_set_vid_mode() sequence, including two msleep(1) delays in
ufx_config_pix_clk().  Once the disconnect callback returns, the USB core
may release the usb_device, and the next ufx_reg_write()/ufx_reg_read()
dereferences dev->udev through __create_pipe():

  general protection fault, probably for non-canonical address
  0xdffffc0000000006: 0000 [#1] SMP
  KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
  RIP: __create_pipe include/linux/usb.h:2031
  Call Trace:
   ufx_set_vid_mode
   ufx_ops_set_par
   fb_set_var
   fb_set_var_from_user
   store_rotate

Take a reference to the usb_device in ufx_usb_probe() and drop it in
ufx_free(), so dev->udev and dev->gdev stay valid until the last
ufx_data user is gone.  The control transfers then fail with -ENODEV
instead of dereferencing freed memory.

Also return early from ufx_set_vid_mode() while the device is inactive,
matching the other transmit paths.  That check alone is not enough,
because it is not serialized against ufx_usb_disconnect() and the
transfer sequence continues after it; it only avoids a long series of
failing transfers.

Fixes: 3c8a63e22a08 ("Add support for SMSC UFX6000/7000 USB display adapters")
Cc: stable@xxxxxxxxxxxxxxx
---
Changes in v2:
 - Hold a reference to the usb_device and drop it in ufx_free() so
   dev->udev stays valid for the whole transfer sequence; the
   usb_active check alone only covers the entry (review feedback).
 drivers/video/fbdev/smscufx.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 5704f60e1741..eac303adb69a 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -661,8 +661,12 @@ static int ufx_set_vid_mode(struct ufx_data *dev, struct fb_var_screeninfo *var)
 	u32 temp;
 	u16 h_total, h_active, h_blank_start, h_blank_end, h_sync_start, h_sync_end;
 	u16 v_total, v_active, v_blank_start, v_blank_end, v_sync_start, v_sync_end;
+	int status;
+
+	if (!atomic_read(&dev->usb_active))
+		return -ENODEV;
 
-	int status = ufx_reg_write(dev, 0x8028, 0);
+	status = ufx_reg_write(dev, 0x8028, 0);
 	check_warn_return(status, "ufx_set_vid_mode error disabling RGB pad");
 
 	status = ufx_reg_write(dev, 0x8024, 0);
@@ -1064,6 +1068,7 @@ static void ufx_free(struct kref *kref)
 {
 	struct ufx_data *dev = container_of(kref, struct ufx_data, kref);
 
+	usb_put_dev(dev->udev);
 	kfree(dev);
 }
 
@@ -1606,6 +1611,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
 
 	dev->udev = usbdev;
 	dev->gdev = &usbdev->dev; /* our generic struct device * */
+	/* keep the device alive until the last ufx_data user is gone */
+	usb_get_dev(usbdev);
 	usb_set_intfdata(interface, dev);
 
 	dev_dbg(dev->gdev, "%s %s - serial #%s\n",
-- 
2.34.1