From 3a01ad8acc2461dc42a88cd80434c8d0ba8099a5 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Tue, 15 Dec 2020 14:24:17 +0000 Subject: [PATCH] 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 Change-Id: I4c839627648c209341a81efa0c001c8d71b878d4 Signed-off-by: Giuliano Procida --- drivers/usb/gadget/function/f_accessory.c | 80 ++++++++++++++++++----- 1 file changed, 62 insertions(+), 18 deletions(-) diff --git a/drivers/usb/gadget/function/f_accessory.c b/drivers/usb/gadget/function/f_accessory.c index fdf55ca81467..5bc32de494c8 100644 --- a/drivers/usb/gadget/function/f_accessory.c +++ b/drivers/usb/gadget/function/f_accessory.c @@ -281,6 +281,15 @@ struct acc_instance { 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); @@ -346,7 +355,10 @@ static void acc_set_disconnected(struct acc_dev *dev) 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"); @@ -356,11 +368,15 @@ static void acc_complete_in(struct usb_ep *ep, struct usb_request *req) 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) { @@ -369,6 +385,7 @@ static void acc_complete_out(struct usb_ep *ep, struct usb_request *req) } wake_up(&dev->read_wq); + put_acc_dev(dev); } static void acc_complete_set_string(struct usb_ep *ep, struct usb_request *req) @@ -836,21 +853,36 @@ static long acc_ioctl(struct file *fp, unsigned code, unsigned long value) 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; } @@ -903,7 +935,7 @@ static void acc_complete_setup_noop(struct usb_ep *ep, struct usb_request *req) 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; @@ -1008,6 +1040,7 @@ int acc_ctrlrequest(struct usb_composite_dev *cdev, "%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); @@ -1078,10 +1111,6 @@ kill_all_hid_devices(struct acc_dev *dev) 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); @@ -1180,12 +1209,15 @@ static void acc_hid_delete(struct acc_hid_dev *hid) 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); @@ -1231,6 +1263,8 @@ static void acc_hid_work(struct work_struct *data) hid_destroy_device(hid->hid); acc_hid_delete(hid); } + + put_acc_dev(dev); } static int acc_function_set_alt(struct usb_function *f, @@ -1323,15 +1357,23 @@ static int acc_setup(void) 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) @@ -1413,7 +1455,9 @@ static struct usb_function_instance *acc_alloc_inst(void) 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, @@ -1426,7 +1470,7 @@ int acc_ctrlrequest_configfs(struct usb_function *f, 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,