From f2a482422fefadfa0fa9b4146fc0e2b46ac04922 Mon Sep 17 00:00:00 2001 From: Liangliang Lu Date: Fri, 5 May 2017 08:50:32 +0800 Subject: net: usb: rmnet_usb_ctrl:Make sure list_head operate atomically Get and delete operation on variables "list_elem" are not atomic. Multiple threads may get the same "list_elem", may lead to race conditions. Add mutex in rmnet_ctl_open to resolve current potential race condition between test_bit and set_bit. Change-Id: I00c4e2fd4854ee17a13a0757da98c46a78eee4cb Signed-off-by: Liangliang Lu --- drivers/net/usb/rmnet_usb_ctrl.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/rmnet_usb_ctrl.c b/drivers/net/usb/rmnet_usb_ctrl.c index 58fd1f6..75e9783 100644 --- a/drivers/net/usb/rmnet_usb_ctrl.c +++ b/drivers/net/usb/rmnet_usb_ctrl.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2011-2014, The Linux Foundation. All rights reserved. +/* Copyright (c) 2011-2014, 2017, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -514,8 +514,13 @@ static int rmnet_ctl_open(struct inode *inode, struct file *file) if (!dev) return -ENODEV; - if (test_bit(RMNET_CTRL_DEV_OPEN, &dev->status)) + mutex_lock(&dev->dev_lock); + if (test_bit(RMNET_CTRL_DEV_OPEN, &dev->status)) { + mutex_unlock(&dev->dev_lock); goto already_opened; + } + set_bit(RMNET_CTRL_DEV_OPEN, &dev->status); + mutex_unlock(&dev->dev_lock); if (dev->mdm_wait_timeout && !test_bit(RMNET_CTRL_DEV_READY, &dev->cudev->status)) { @@ -527,10 +532,15 @@ static int rmnet_ctl_open(struct inode *inode, struct file *file) if (retval == 0) { dev_err(dev->devicep, "%s: Timeout opening %s\n", __func__, dev->name); - return -ETIMEDOUT; - } else if (retval < 0) { + retval = -ETIMEDOUT; + } else if (retval < 0) dev_err(dev->devicep, "%s: Error waiting for %s\n", __func__, dev->name); + + if (retval < 0) { + mutex_lock(&dev->dev_lock); + clear_bit(RMNET_CTRL_DEV_OPEN, &dev->status); + mutex_unlock(&dev->dev_lock); return retval; } } @@ -538,14 +548,15 @@ static int rmnet_ctl_open(struct inode *inode, struct file *file) if (!test_bit(RMNET_CTRL_DEV_READY, &dev->cudev->status)) { dev_dbg(dev->devicep, "%s: Connection timedout opening %s\n", __func__, dev->name); + mutex_lock(&dev->dev_lock); + clear_bit(RMNET_CTRL_DEV_OPEN, &dev->status); + mutex_unlock(&dev->dev_lock); return -ETIMEDOUT; } /* clear stale data if device close called but channel was ready */ rmnet_usb_ctrl_free_rx_list(dev); - set_bit(RMNET_CTRL_DEV_OPEN, &dev->status); - file->private_data = dev; already_opened: @@ -564,7 +575,9 @@ static int rmnet_ctl_release(struct inode *inode, struct file *file) DBG("%s Called on %s device\n", __func__, dev->name); + mutex_lock(&dev->dev_lock); clear_bit(RMNET_CTRL_DEV_OPEN, &dev->status); + mutex_unlock(&dev->dev_lock); file->private_data = NULL; @@ -638,6 +651,7 @@ ctrl_read: list_elem = list_first_entry(&dev->rx_list, struct ctrl_pkt_list_elem, list); + list_del(&list_elem->list); bytes_to_read = (uint32_t)(list_elem->cpkt.data_size); if (bytes_to_read > count) { spin_unlock_irqrestore(&dev->rx_lock, flags); @@ -654,11 +668,11 @@ ctrl_read: dev_err(dev->devicep, "%s: copy_to_user failed for %s\n", __func__, dev->name); + spin_lock_irqsave(&dev->rx_lock, flags); + list_add(&list_elem->list, &dev->rx_list); + spin_unlock_irqrestore(&dev->rx_lock, flags); return -EFAULT; } - spin_lock_irqsave(&dev->rx_lock, flags); - list_del(&list_elem->list); - spin_unlock_irqrestore(&dev->rx_lock, flags); kfree(list_elem->cpkt.data); kfree(list_elem); -- cgit v1.1