Skip to contentSkip to conversation

media: dvb-core: Fix use-after-free due to race condition occurring in dvb_ca_en50221

PATCH In review Hyunwoo Kim 2 participants 2 messages Nov 21, 2022 – Jan 10, 2023

Subsystems (2)
Media (V4L/DVB) USB
lore
Share & tools
lore

Hyunwoo Kim

author
#0 lore raw

PATCH 1/1 media: dvb-core: Fix use-after-free due to race condition occurring in dvb_ca_en50221

If the device node of dvb_ca_en50221 is open() and the device is disconnected, a UAF may occur when calling close() on the device node.

The root cause is that wake_up() and wait_event() for dvbdev->wait_queue are not implemented.

So implement wait_event() function in dvb_ca_en50221_release() and add 'remove_mutex' which prevents race condition for 'ca->exit'.

Signed-off-by Hyunwoo Kim <[email protected]>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 36 ++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)
drivers/media/dvb-core/dvb_ca_en50221.c +35 -1
index 15a08d8c69ef..60133a315701 100644
@@ -151,6 +151,12 @@ struct dvb_ca_private {
 
 	/* mutex serializing ioctls */
 	struct mutex ioctl_mutex;
+
+	/* A mutex used when a device is disconnected */
+	struct mutex remove_mutex;
+
+	/* Whether the device is disconnected */
+	int exit;
 };
 
 static void dvb_ca_private_free(struct dvb_ca_private *ca)

@@ -1709,12 +1715,22 @@ static int dvb_ca_en50221_io_open(struct inode *inode, struct file *file)
 
 	dprintk("%s\n", __func__);
 
-	if (!try_module_get(ca->pub->owner))
+	mutex_lock(&ca->remove_mutex);
+
+	if (ca->exit) {
+		mutex_unlock(&ca->remove_mutex);
+		return -ENODEV;
+	}
+
+	if (!try_module_get(ca->pub->owner)) {
+		mutex_unlock(&ca->remove_mutex);
 		return -EIO;
+	}
 
 	err = dvb_generic_open(inode, file);
 	if (err < 0) {
 		module_put(ca->pub->owner);
+		mutex_unlock(&ca->remove_mutex);
 		return err;
 	}
 

@@ -1739,6 +1755,7 @@ static int dvb_ca_en50221_io_open(struct inode *inode, struct file *file)
 
 	dvb_ca_private_get(ca);
 
+	mutex_unlock(&ca->remove_mutex);
 	return 0;
 }
 

@@ -1758,6 +1775,8 @@ static int dvb_ca_en50221_io_release(struct inode *inode, struct file *file)
 
 	dprintk("%s\n", __func__);
 
+	mutex_lock(&ca->remove_mutex);
+
 	/* mark the CA device as closed */
 	ca->open = 0;
 	dvb_ca_en50221_thread_update_delay(ca);

@@ -1768,6 +1787,12 @@ static int dvb_ca_en50221_io_release(struct inode *inode, struct file *file)
 
 	dvb_ca_private_put(ca);
 
+	if (dvbdev->users == 1 && ca->exit == 1) {
+		mutex_unlock(&ca->remove_mutex);
+		wake_up(&dvbdev->wait_queue);
+	} else
+		mutex_unlock(&ca->remove_mutex);
+
 	return err;
 }
 

@@ -1891,6 +1916,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 	}
 
 	mutex_init(&ca->ioctl_mutex);
+	mutex_init(&ca->remove_mutex);
 
 	if (signal_pending(current)) {
 		ret = -EINTR;

@@ -1933,6 +1959,14 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 
 	dprintk("%s\n", __func__);
 
+	mutex_lock(&ca->remove_mutex);
+	ca->exit = 1;
+	mutex_unlock(&ca->remove_mutex);
+
+	if (ca->dvbdev->users < 1)
+		wait_event(ca->dvbdev->wait_queue,
+				ca->dvbdev->users == 1);
+
 	/* shutdown the thread if there was one */
 	kthread_stop(ca->thread);
Signature · 29 lines
2.25.1


Dear,

A UAF can occur in a flow like the one below:
```
                cpu0                                                cpu1
                                                             1. dvb_usbv2_probe()
                                                                d->priv = kzalloc(d->props->size_of_priv, GFP_KERNEL);
                                                                ...
                                                                dvb_usbv2_init()
                                                                anysee_init()
                                                                anysee_ci_init()
                                                                dvb_ca_en50221_init()
                                                                ca->pub = pubca;    // pubca : &state->ci, state : d->priv
       2. open()
          dvb_device_open()
          dvb_ca_en50221_io_open()
                                                             3. dvb_usbv2_disconnect()
                                                                kfree(d->priv);
       4. close()
          dvb_ca_en50221_io_release()
          module_put(ca->pub->owner);    // UAF
```


Regards,
Hyunwoo Kim
#1 lore raw
Quoted 5 lines · Hyunwoo Kim

If the device node of dvb_ca_en50221 is open() and the device is disconnected, a UAF may occur when calling close() on the device node.

Quoted 8 lines

The root cause is that wake_up() and wait_event() for dvbdev->wait_queue are not implemented.

So implement wait_event() function in dvb_ca_en50221_release() and add 'remove_mutex' which prevents race condition for 'ca->exit'.

Signed-off-by Hyunwoo Kim <[email protected]>

Just wonder what happens on this. Is this still persistent with the latest upstream kernel?

Note that CVE-2022-45919 has been assigned to this bug.

thanks,

Takashi