usbip_sockfd_store() must perform if (udc->ud.status != SDEV_ST_AVAILABLE) { /* misc assignments for attach operation */ udc->ud.status = SDEV_ST_USED; } under a lock, or multiple ud->tcp_{tx,rx} are created (which will later cause a crash like [1]) and refcount on ud->tcp_socket is leaked when usbip_sockfd_store() is concurrently called. Since I don't want to replace "return" with "goto" in order to call usbip_event_unlock(), wrap whole function in order to keep changes minimal. [1] https://syzkaller.appspot.com/bug?extid=95ce4b142579611ef0a9 Reported-by: syzbot <syzbot+95ce4b142579611ef0a9@xxxxxxxxxxxxxxxxxxxxxxxxx> Tested-by: syzbot <syzbot+a93fba6d384346a761e3@xxxxxxxxxxxxxxxxxxxxxxxxx> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> --- drivers/usb/usbip/vudc_sysfs.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/usb/usbip/vudc_sysfs.c b/drivers/usb/usbip/vudc_sysfs.c index 100f680c572a..ebf6a127fcc3 100644 --- a/drivers/usb/usbip/vudc_sysfs.c +++ b/drivers/usb/usbip/vudc_sysfs.c @@ -90,8 +90,8 @@ static ssize_t dev_desc_read(struct file *file, struct kobject *kobj, } static BIN_ATTR_RO(dev_desc, sizeof(struct usb_device_descriptor)); -static ssize_t usbip_sockfd_store(struct device *dev, struct device_attribute *attr, - const char *in, size_t count) +static ssize_t __usbip_sockfd_store(struct device *dev, struct device_attribute *attr, + const char *in, size_t count) { struct vudc *udc = (struct vudc *) dev_get_drvdata(dev); int rv; @@ -184,6 +184,17 @@ static ssize_t usbip_sockfd_store(struct device *dev, struct device_attribute *a return ret; } +static ssize_t usbip_sockfd_store(struct device *dev, struct device_attribute *attr, + const char *in, size_t count) +{ + ssize_t ret = usbip_event_lock_killable(); + + if (ret) + return ret; + ret = __usbip_sockfd_store(dev, attr, in, count); + usbip_event_unlock(); + return ret; +} static DEVICE_ATTR_WO(usbip_sockfd); static ssize_t usbip_status_show(struct device *dev, -- 2.18.4