ANDROID: usb: f_accessory: Wrap '_acc_dev' in get()/put() accessors
The '_acc_dev' global variable is a fancy use-after-free factory. Wrap
it in some get()/put() functions in preparation for introducing some
refcounting.
Bug: 173789633
Signed-off-by: Will Deacon <willdeacon@google.com>
Change-Id: I4c839627648c209341a81efa0c001c8d71b878d4
Signed-off-by: Giuliano Procida <gprocida@google.com>
diff --git a/drivers/usb/gadget/function/f_accessory.c b/drivers/usb/gadget/function/f_accessory.c
index 59831d7..b91cd38 100644
--- a/drivers/usb/gadget/function/f_accessory.c
+++ b/drivers/usb/gadget/function/f_accessory.c
@@ -206,6 +206,15 @@
const char *name;
};
+static struct acc_dev *get_acc_dev(void)
+{
+ return _acc_dev;
+}
+
+static void put_acc_dev(struct acc_dev *dev)
+{
+}
+
static inline struct acc_dev *func_to_dev(struct usb_function *f)
{
return container_of(f, struct acc_dev, function);
@@ -271,7 +280,10 @@
static void acc_complete_in(struct usb_ep *ep, struct usb_request *req)
{
- struct acc_dev *dev = _acc_dev;
+ struct acc_dev *dev = get_acc_dev();
+
+ if (!dev)
+ return;
if (req->status == -ESHUTDOWN) {
pr_debug("acc_complete_in set disconnected");
@@ -281,11 +293,15 @@
req_put(dev, &dev->tx_idle, req);
wake_up(&dev->write_wq);
+ put_acc_dev(dev);
}
static void acc_complete_out(struct usb_ep *ep, struct usb_request *req)
{
- struct acc_dev *dev = _acc_dev;
+ struct acc_dev *dev = get_acc_dev();
+
+ if (!dev)
+ return;
dev->rx_done = 1;
if (req->status == -ESHUTDOWN) {
@@ -294,6 +310,7 @@
}
wake_up(&dev->read_wq);
+ put_acc_dev(dev);
}
static void acc_complete_set_string(struct usb_ep *ep, struct usb_request *req)
@@ -761,21 +778,36 @@
static int acc_open(struct inode *ip, struct file *fp)
{
- if (atomic_xchg(&_acc_dev->open_excl, 1))
- return -EBUSY;
+ struct acc_dev *dev = get_acc_dev();
- _acc_dev->disconnected = 0;
- fp->private_data = _acc_dev;
+ if (!dev)
+ return -ENODEV;
+
+ if (atomic_xchg(&dev->open_excl, 1)) {
+ put_acc_dev(dev);
+ return -EBUSY;
+ }
+
+ dev->disconnected = 0;
+ fp->private_data = dev;
return 0;
}
static int acc_release(struct inode *ip, struct file *fp)
{
- WARN_ON(!atomic_xchg(&_acc_dev->open_excl, 0));
+ struct acc_dev *dev = fp->private_data;
+
+ if (!dev)
+ return -ENOENT;
+
+ WARN_ON(!atomic_xchg(&dev->open_excl, 0));
/* indicate that we are disconnected
* still could be online so don't touch online flag
*/
- _acc_dev->disconnected = 1;
+ dev->disconnected = 1;
+
+ fp->private_data = NULL;
+ put_acc_dev(dev);
return 0;
}
@@ -828,7 +860,7 @@
int acc_ctrlrequest(struct usb_composite_dev *cdev,
const struct usb_ctrlrequest *ctrl)
{
- struct acc_dev *dev = _acc_dev;
+ struct acc_dev *dev = get_acc_dev();
int value = -EOPNOTSUPP;
struct acc_hid_dev *hid;
int offset;
@@ -931,6 +963,7 @@
"%02x.%02x v%04x i%04x l%u\n",
ctrl->bRequestType, ctrl->bRequest,
w_value, w_index, w_length);
+ put_acc_dev(dev);
return value;
}
EXPORT_SYMBOL_GPL(acc_ctrlrequest);
@@ -1001,10 +1034,6 @@
struct list_head *entry, *temp;
unsigned long flags;
- /* do nothing if usb accessory device doesn't exist */
- if (!dev)
- return;
-
spin_lock_irqsave(&dev->lock, flags);
list_for_each_safe(entry, temp, &dev->hid_list) {
hid = list_entry(entry, struct acc_hid_dev, list);
@@ -1089,12 +1118,15 @@
static void acc_hid_work(struct work_struct *data)
{
- struct acc_dev *dev = _acc_dev;
+ struct acc_dev *dev = get_acc_dev();
struct list_head *entry, *temp;
struct acc_hid_dev *hid;
struct list_head new_list, dead_list;
unsigned long flags;
+ if (!dev)
+ return;
+
INIT_LIST_HEAD(&new_list);
spin_lock_irqsave(&dev->lock, flags);
@@ -1140,6 +1172,8 @@
hid_destroy_device(hid->hid);
acc_hid_delete(hid);
}
+
+ put_acc_dev(dev);
}
static int acc_function_set_alt(struct usb_function *f,
@@ -1230,15 +1264,23 @@
void acc_disconnect(void)
{
+ struct acc_dev *dev = get_acc_dev();
+
/* unregister all HID devices if USB is disconnected */
- kill_all_hid_devices(_acc_dev);
+ if (dev)
+ kill_all_hid_devices(dev);
+
+ put_acc_dev(dev);
}
EXPORT_SYMBOL_GPL(acc_disconnect);
static void acc_cleanup(void)
{
+ struct acc_dev *dev = _acc_dev;
+
misc_deregister(&acc_device);
- kfree(_acc_dev);
+ put_acc_dev(dev);
+ kfree(dev);
_acc_dev = NULL;
}
static struct acc_instance *to_acc_instance(struct config_item *item)
@@ -1320,7 +1362,9 @@
static void acc_free(struct usb_function *f)
{
-/*NO-OP: no function specific resource allocation in mtp_alloc*/
+ struct acc_dev *dev = func_to_dev(f);
+
+ put_acc_dev(dev);
}
int acc_ctrlrequest_configfs(struct usb_function *f,
@@ -1333,7 +1377,7 @@
static struct usb_function *acc_alloc(struct usb_function_instance *fi)
{
- struct acc_dev *dev = _acc_dev;
+ struct acc_dev *dev = get_acc_dev();
dev->function.name = "accessory";
dev->function.strings = acc_strings,
