diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index d7aa5511c361..a4378a7241f7 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -137,6 +137,8 @@ static int ufx_submit_urb(struct ufx_data *dev, struct urb * urb, size_t len);
 static int ufx_alloc_urb_list(struct ufx_data *dev, int count, size_t size);
 static void ufx_free_urb_list(struct ufx_data *dev);
 
+static DEFINE_MUTEX(disconnect_mutex);
+
 /* reads a control register */
 static int ufx_reg_read(struct ufx_data *dev, u32 index, u32 *data)
 {
@@ -1065,6 +1067,8 @@ static int ufx_ops_open(struct fb_info *info, int user)
 {
 	struct ufx_data *dev = info->par;
 
+	mutex_lock(&disconnect_mutex);
+
 	/* fbcon aggressively connects to first framebuffer it finds,
 	 * preventing other clients (X) from working properly. Usually
 	 * not what the user wants. Fail by default with option to enable. */
@@ -1097,6 +1101,8 @@ static int ufx_ops_open(struct fb_info *info, int user)
 	pr_debug("open /dev/fb%d user=%d fb_info=%p count=%d",
 		info->node, user, info, dev->fb_count);
 
+	mutex_unlock(&disconnect_mutex);
+
 	return 0;
 }
 
@@ -1741,6 +1747,8 @@ static void ufx_usb_disconnect(struct usb_interface *interface)
 {
 	struct ufx_data *dev;
 
+	mutex_lock(&disconnect_mutex);
+
 	dev = usb_get_intfdata(interface);
 
 	pr_debug("USB disconnect starting\n");
@@ -1761,6 +1769,8 @@ static void ufx_usb_disconnect(struct usb_interface *interface)
 	kref_put(&dev->kref, ufx_free);
 
 	/* consider ufx_data freed */
+
+	mutex_unlock(&disconnect_mutex);
 }
 
 static struct usb_driver ufx_driver = {
-- 
2.25.1


Dear all,


I found a race-condition-to-UAF vulnerability in "drivers/video/fbdev/smscufx.c".


# Introduction

This vulnerability is a race condition between the ufx_ops_open() and ufx_usb_disconnect() functions.

Because this driver uses kref to manage the "dev" structure, it looks like a race condition will not occur, 
but in reality, a race condition occurs because there is no lock between the above two functions:
```
                cpu0                                                cpu1
       1. open()
          ufx_ops_open()
          if (dev->virtualized)
                                                             2. ufx_usb_disconnect()
                                                                dev->virtualized = true;
                                                                atomic_set()
                                                                usb_set_intfdata()

                                                             3. if (dev->fb_count == 0)
                                                                schedule_delayed_work(&dev->free_framebuffer_work, 0)
                                                                kref_put()   <- kref count : 1
                                                                kref_put()   <- kref count : 0
                                                                ufx_free()
                                                                kfree(dev);

       4. dev->fb_count++;   <- UAF start
          kref_get()   <- refcount_t: addition on 0; use-after-free.
```
The detailed exploit flow is as follows:

1. open() the device node.
dev->virtualized set to "true" only in ufx_usb_disconnect(),
It passes "if (dev->virtualized)" because it hasn't been set yet.

2. ufx_usb_disconnect() is called by physically removing the usb device.
"dev->virtualized = true;" is executed. 
But checking this flag doesn't make any sense because step 1 has already passed.
1 ~ 2 are the first race conditions.

3. After "if (dev->fb_count == 0)" is passed, call kref_put() twice with delayed queue. 
Eventually "kfree(dev);" is executed.

4. "dev->fb_count++" is called. 
However, at this point "dev" is kfree()d, so it becomes a UAF.
3 ~ 4 are the second race conditions.
After this, many UAF scenarios can be configured.


However, if you actually check the code, the workload of 1 ~ 4 of open() is much shorter 
than the workload of 2 ~ 3 of disconnect().
As a result, it is almost impossible to trigger this race condition with a brute force.

Here, an attacker can successfully achieve a race condition using the technique introduced in this paper:
https://www.usenix.org/system/files/sec21-lee-yoochan.pdf

To briefly summarize the paper, 
the processing time between 1 and 4 of open() can be extended longer by using Reschedule IPI.
In other words, it can dramatically increase the success probability of the above race condition scenario.


# for debugging

To debug this race condition, it is convenient to modify the code as follows:
```
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index d7aa5511c361..bbb42e9e768d 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -137,6 +137,10 @@ static int ufx_submit_urb(struct ufx_data *dev, struct urb * urb, size_t len);
 static int ufx_alloc_urb_list(struct ufx_data *dev, int count, size_t size);
 static void ufx_free_urb_list(struct ufx_data *dev);

+DECLARE_WAIT_QUEUE_HEAD(race_condition_debugq);
+static int race_check = 0;
+static struct ufx_data *open_dev;
+
 /* reads a control register */
 static int ufx_reg_read(struct ufx_data *dev, u32 index, u32 *data)
 {
@@ -1075,6 +1079,9 @@ static int ufx_ops_open(struct fb_info *info, int user)
        if (dev->virtualized)
                return -ENODEV;

+       open_dev = dev;
+       wait_event(race_condition_debugq, (race_check == 1));
+
        dev->fb_count++;

        kref_get(&dev->kref);
@@ -1760,6 +1767,12 @@ static void ufx_usb_disconnect(struct usb_interface *interface)
        /* release reference taken by kref_init in probe() */
        kref_put(&dev->kref, ufx_free);

+       if (open_dev == dev) {
+                usleep_range(1000000, 1000001);
+                race_check = 1;
+                wake_up(&race_condition_debugq);
+        }
+
        /* consider ufx_data freed */
 }
```
