Skip to content

Commit c024f2c

Browse files
Rohith Kollalsichrmhoffmann
authored andcommitted
usb: gadget: f_cdev: Fix use after free of port in f_cdev
With the configfs filesystem it’s possible to manipulate kernel object by creating/deleting folders into /config path. Here port object is created by a mkdir and leads to allocate this object, while the rmdir system call leads to free this object. If one thread does these two operations of creation and deletion of the folder and one tries to open it, it can lead to a race condition where port object can be freed by the time it is used in f_cdev_open leading to use after free error. Fix this by using embedded struct device and the refcounting mechanism built-in which increases and decreases refcount upon creation and deletion of port and port will be freed when reference count is zero ensuring that "port" object survives until the last user releases it. Bug: 178720043 Change-Id: I88701ef161c9f3215631da81c3a8d4c980d12b25 Signed-off-by: Rohith Kollalsi <rkollals@codeaurora.org>
1 parent 57144df commit c024f2c

File tree

1 file changed

+28
-28
lines changed

1 file changed

+28
-28
lines changed

drivers/usb/gadget/function/f_cdev.c

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2013-2017, The Linux Foundation. All rights reserved.
2+
* Copyright (c) 2011, 2013-2021, The Linux Foundation. All rights reserved.
33
* Linux Foundation chooses to take subject only to the GPLv2 license terms,
44
* and distributes only under these terms.
55
*
@@ -91,7 +91,7 @@ struct cserial {
9191

9292
struct f_cdev {
9393
struct cdev fcdev_cdev;
94-
struct device *dev;
94+
struct device dev;
9595
unsigned port_num;
9696
char name[sizeof(DEVICE_NAME) + 2];
9797
int minor;
@@ -824,12 +824,15 @@ static void cser_free_inst(struct usb_function_instance *fi)
824824
opts = container_of(fi, struct f_cdev_opts, func_inst);
825825

826826
if (opts->port) {
827-
device_destroy(fcdev_classp, MKDEV(major, opts->port->minor));
828-
cdev_del(&opts->port->fcdev_cdev);
827+
cdev_device_del(&opts->port->fcdev_cdev, &opts->port->dev);
828+
mutex_lock(&chardev_ida_lock);
829+
ida_simple_remove(&chardev_ida, opts->port->minor);
830+
mutex_unlock(&chardev_ida_lock);
831+
put_device(&opts->port->dev);
829832
}
833+
830834
usb_cser_chardev_deinit();
831835
kfree(opts->func_name);
832-
kfree(opts->port);
833836
kfree(opts);
834837
}
835838

@@ -1050,13 +1053,10 @@ int f_cdev_open(struct inode *inode, struct file *file)
10501053
struct f_cdev *port;
10511054

10521055
port = container_of(inode->i_cdev, struct f_cdev, fcdev_cdev);
1053-
if (!port) {
1054-
pr_err("Port is NULL.\n");
1055-
return -EINVAL;
1056-
}
1057-
1058-
if (port && port->port_open) {
1056+
get_device(&port->dev);
1057+
if (port->port_open) {
10591058
pr_err("port is already opened.\n");
1059+
put_device(&port->dev);
10601060
return -EBUSY;
10611061
}
10621062

@@ -1066,6 +1066,7 @@ int f_cdev_open(struct inode *inode, struct file *file)
10661066
port->is_connected);
10671067
if (ret) {
10681068
pr_debug("open interrupted.\n");
1069+
put_device(&port->dev);
10691070
return ret;
10701071
}
10711072

@@ -1085,16 +1086,12 @@ int f_cdev_release(struct inode *inode, struct file *file)
10851086
struct f_cdev *port;
10861087

10871088
port = file->private_data;
1088-
if (!port) {
1089-
pr_err("port is NULL.\n");
1090-
return -EINVAL;
1091-
}
1092-
10931089
spin_lock_irqsave(&port->port_lock, flags);
10941090
port->port_open = false;
10951091
port->cbits_updated = false;
10961092
spin_unlock_irqrestore(&port->port_lock, flags);
10971093
pr_debug("port(%s)(%pK) is closed.\n", port->name, port);
1094+
put_device(&port->dev);
10981095

10991096
return 0;
11001097
}
@@ -1507,11 +1504,17 @@ static const struct file_operations f_cdev_fops = {
15071504
.compat_ioctl = f_cdev_ioctl,
15081505
};
15091506

1507+
static void cdev_device_release(struct device *dev)
1508+
{
1509+
struct f_cdev *port = container_of(dev, struct f_cdev, dev);
1510+
1511+
pr_debug("Free cdev port(%d)\n", port->port_num);
1512+
kfree(port);
1513+
}
1514+
15101515
static struct f_cdev *f_cdev_alloc(char *func_name, int portno)
15111516
{
15121517
int ret;
1513-
dev_t dev;
1514-
struct device *device;
15151518
struct f_cdev *port;
15161519

15171520
port = kzalloc(sizeof(struct f_cdev), GFP_KERNEL);
@@ -1561,25 +1564,22 @@ static struct f_cdev *f_cdev_alloc(char *func_name, int portno)
15611564

15621565
/* create char device */
15631566
cdev_init(&port->fcdev_cdev, &f_cdev_fops);
1564-
dev = MKDEV(major, port->minor);
1565-
ret = cdev_add(&port->fcdev_cdev, dev, 1);
1567+
device_initialize(&port->dev);
1568+
port->dev.class = fcdev_classp;
1569+
port->dev.parent = NULL;
1570+
port->dev.release = cdev_device_release;
1571+
port->dev.devt = MKDEV(major, port->minor);
1572+
dev_set_name(&port->dev, port->name);
1573+
ret = cdev_device_add(&port->fcdev_cdev, &port->dev);
15661574
if (ret) {
15671575
pr_err("Failed to add cdev for port(%s)\n", port->name);
15681576
goto err_cdev_add;
15691577
}
15701578

1571-
device = device_create(fcdev_classp, NULL, dev, NULL, port->name);
1572-
if (IS_ERR(device)) {
1573-
ret = PTR_ERR(device);
1574-
goto err_create_dev;
1575-
}
1576-
15771579
pr_info("port_name:%s (%pK) portno:(%d)\n",
15781580
port->name, port, port->port_num);
15791581
return port;
15801582

1581-
err_create_dev:
1582-
cdev_del(&port->fcdev_cdev);
15831583
err_cdev_add:
15841584
destroy_workqueue(port->fcdev_wq);
15851585
err_get_ida:

0 commit comments

Comments
 (0)