From b2fa897c8e86362946ec524ed47300164a33453d Mon Sep 17 00:00:00 2001 From: Lena Salman Date: Wed, 14 May 2014 10:59:58 +0300 Subject: [PATCH] USB: f_qc_rndis: Prevent use-after-free for _rndis_qc Assume that there are two threads, thread1 is setting value of _rndis_qc variable in rndis_qc_bind_config_vendor function. Thread2 jumps in and get the value of _rndis_qc in rndis_qc_open_dev function before it is freed in rndis_qc_bind_config_vendor function, since rndis_ipa_init or usb_add_function failed. Use-after-free will happen as Thread2 is referencing freed objects. To prevent this spinlock is used where ever it is needed to protect _rndis_qc variable. Bug: 35136547 Change-Id: Ib45ae14281821eeaf79419e8d177cb5d51b94df8 --- drivers/usb/gadget/f_qc_rndis.c | 105 +++++++++++++++++++++++++++++----------- 1 file changed, 76 insertions(+), 29 deletions(-) diff --git a/drivers/usb/gadget/f_qc_rndis.c b/drivers/usb/gadget/f_qc_rndis.c index 819bde5072a39..b7f37df6921d8 100644 --- a/drivers/usb/gadget/f_qc_rndis.c +++ b/drivers/usb/gadget/f_qc_rndis.c @@ -81,7 +81,7 @@ */ struct f_rndis_qc { - struct qc_gether port; + struct qc_gether port; u8 ctrl_id, data_id; u8 ethaddr[ETH_ALEN]; u32 vendorID; @@ -90,8 +90,8 @@ struct f_rndis_qc { u32 max_pkt_size; const char *manufacturer; int config; - atomic_t ioctl_excl; - atomic_t open_excl; + atomic_t ioctl_excl; + atomic_t open_excl; struct usb_ep *notify; struct usb_request *notify_req; @@ -101,6 +101,7 @@ struct f_rndis_qc { }; static struct ipa_usb_init_params rndis_ipa_params; +static spinlock_t rndis_lock; static bool rndis_ipa_supported; static void rndis_qc_open(struct qc_gether *geth); @@ -548,7 +549,7 @@ static void rndis_qc_response_available(void *_rndis) } static void rndis_qc_response_complete(struct usb_ep *ep, - struct usb_request *req) + struct usb_request *req) { struct f_rndis_qc *rndis = req->context; int status = req->status; @@ -693,7 +694,7 @@ rndis_qc_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl) static int rndis_qc_set_alt(struct usb_function *f, unsigned intf, unsigned alt) { - struct f_rndis_qc *rndis = func_to_rndis_qc(f); + struct f_rndis_qc *rndis = func_to_rndis_qc(f); struct usb_composite_dev *cdev = f->config->cdev; /* we know alt == 0 */ @@ -1033,6 +1034,7 @@ static void rndis_qc_unbind(struct usb_configuration *c, struct usb_function *f) { struct f_rndis_qc *rndis = func_to_rndis_qc(f); + unsigned long flags; pr_debug("rndis_qc_unbind: free"); bam_data_destroy(0); @@ -1051,7 +1053,10 @@ rndis_qc_unbind(struct usb_configuration *c, struct usb_function *f) rndis_ipa_supported = false; } + spin_lock_irqsave(&rndis_lock, flags); kfree(rndis); + _rndis_qc = NULL; + spin_unlock_irqrestore(&rndis_lock, flags); } bool is_rndis_ipa_supported(void) @@ -1204,8 +1209,6 @@ rndis_qc_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], rndis->port.func.suspend = rndis_qc_suspend; rndis->port.func.resume = rndis_qc_resume; - _rndis_qc = rndis; - if (rndis->xport == USB_GADGET_XPORT_BAM2BAM_IPA) { status = rndis_ipa_init(&rndis_ipa_params); if (status) { @@ -1221,86 +1224,128 @@ rndis_qc_bind_config_vendor(struct usb_configuration *c, u8 ethaddr[ETH_ALEN], goto fail; } - if (rndis->xport != USB_GADGET_XPORT_BAM2BAM_IPA) - return status; + _rndis_qc = rndis; + + return status; fail: - kfree(rndis); - _rndis_qc = NULL; + kfree(rndis); + _rndis_qc = NULL; rndis_exit(); return status; } static int rndis_qc_open_dev(struct inode *ip, struct file *fp) { + int ret = 0; + unsigned long flags; pr_info("Open rndis QC driver\n"); + spin_lock_irqsave(&rndis_lock, flags); if (!_rndis_qc) { pr_err("rndis_qc_dev not created yet\n"); - return -ENODEV; + ret = -ENODEV; + goto fail; } if (rndis_qc_lock(&_rndis_qc->open_excl)) { pr_err("Already opened\n"); - return -EBUSY; + ret = -EBUSY; + goto fail; } fp->private_data = _rndis_qc; - pr_info("rndis QC file opened\n"); +fail: + spin_unlock_irqrestore(&rndis_lock, flags); - return 0; + if (!ret) + pr_info("rndis QC file opened\n"); + + return ret; } static int rndis_qc_release_dev(struct inode *ip, struct file *fp) { - struct f_rndis_qc *rndis = fp->private_data; - + unsigned long flags; pr_info("Close rndis QC file"); - rndis_qc_unlock(&rndis->open_excl); + spin_lock_irqsave(&rndis_lock, flags); + + if (!_rndis_qc) { + pr_err("rndis_qc_dev not present\n"); + spin_unlock_irqrestore(&rndis_lock, flags); + return -ENODEV; + } + rndis_qc_unlock(&_rndis_qc->open_excl); + spin_unlock_irqrestore(&rndis_lock, flags); return 0; } static long rndis_qc_ioctl(struct file *fp, unsigned cmd, unsigned long arg) { - struct f_rndis_qc *rndis = fp->private_data; + u8 qc_max_pkt_per_xfer = 0; + u32 qc_max_pkt_size = 0; int ret = 0; + unsigned long flags; + + spin_lock_irqsave(&rndis_lock, flags); + if (!_rndis_qc) { + pr_err("rndis_qc_dev not present\n"); + ret = -ENODEV; + goto fail; + } - pr_info("Received command %d", cmd); + qc_max_pkt_per_xfer = _rndis_qc->max_pkt_per_xfer; + qc_max_pkt_size = _rndis_qc->max_pkt_size; - if (rndis_qc_lock(&rndis->ioctl_excl)) - return -EBUSY; + if (rndis_qc_lock(&_rndis_qc->ioctl_excl)) { + ret = -EBUSY; + goto fail; + } + + spin_unlock_irqrestore(&rndis_lock, flags); + + pr_info("Received command %d\n", cmd); switch (cmd) { case RNDIS_QC_GET_MAX_PKT_PER_XFER: ret = copy_to_user((void __user *)arg, - &rndis->max_pkt_per_xfer, - sizeof(rndis->max_pkt_per_xfer)); + &qc_max_pkt_per_xfer, + sizeof(qc_max_pkt_per_xfer)); if (ret) { pr_err("copying to user space failed"); ret = -EFAULT; } pr_info("Sent max packets per xfer %d", - rndis->max_pkt_per_xfer); + qc_max_pkt_per_xfer); break; case RNDIS_QC_GET_MAX_PKT_SIZE: ret = copy_to_user((void __user *)arg, - &rndis->max_pkt_size, - sizeof(rndis->max_pkt_size)); + &qc_max_pkt_size, + sizeof(qc_max_pkt_size)); if (ret) { pr_err("copying to user space failed"); ret = -EFAULT; } pr_debug("Sent max packet size %d", - rndis->max_pkt_size); + qc_max_pkt_size); break; default: pr_err("Unsupported IOCTL"); ret = -EINVAL; } - rndis_qc_unlock(&rndis->ioctl_excl); + spin_lock_irqsave(&rndis_lock, flags); + + if (!_rndis_qc) { + pr_err("rndis_qc_dev not present\n"); + ret = -ENODEV; + goto fail; + } + rndis_qc_unlock(&_rndis_qc->ioctl_excl); +fail: + spin_unlock_irqrestore(&rndis_lock, flags); return ret; } @@ -1323,6 +1368,8 @@ static int rndis_qc_init(void) pr_info("initialize rndis QC instance\n"); + spin_lock_init(&rndis_lock); + ret = misc_register(&rndis_qc_device); if (ret) pr_err("rndis QC driver failed to register");