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: [email protected]
---
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
