| Message ID | 20210416205319.14075-1-tseewald@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] usbip: add sysfs_lock to synchronize sysfs code paths | expand |
On 4/16/21 2:53 PM, Tom Seewald wrote: > From: Shuah Khan <skhan@linuxfoundation.org> > > commit 9dbf34a834563dada91366c2ac266f32ff34641a upstream. > > Fuzzing uncovered race condition between sysfs code paths in usbip > drivers. Device connect/disconnect code paths initiated through > sysfs interface are prone to races if disconnect happens during > connect and vice versa. > > Use sysfs_lock to protect sysfs paths in stub-dev. > > Cc: stable@vger.kernel.org # 4.9.x > Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > Link: https://lore.kernel.org/r/2b182f3561b4a065bf3bf6dce3b0e9944ba17b3f.1616807117.git.skhan@linuxfoundation.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Tom Seewald <tseewald@gmail.com> > --- Thank you for the backport. Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> Greg, please pick this up for 4.9.x thanks, -- Shuah
On 4/16/21 2:53 PM, Tom Seewald wrote: > From: Shuah Khan <skhan@linuxfoundation.org> > > commit 363eaa3a450abb4e63bd6e3ad79d1f7a0f717814 upstream. > > Fuzzing uncovered race condition between sysfs code paths in usbip > drivers. Device connect/disconnect code paths initiated through > sysfs interface are prone to races if disconnect happens during > connect and vice versa. > > Use sysfs_lock to synchronize event handler with sysfs paths > in usbip drivers. > > Cc: stable@vger.kernel.org # 4.9.x > Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > Link: https://lore.kernel.org/r/c5c8723d3f29dfe3d759cfaafa7dd16b0dfe2918.1616807117.git.skhan@linuxfoundation.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Tom Seewald <tseewald@gmail.com> > --- > drivers/usb/usbip/usbip_event.c | 2 ++ > 1 file changed, 2 insertions(+) > Thank you for the backport. Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> Greg, please pick this up for 4.9.x thanks, -- Shuah
On Fri, Apr 16, 2021 at 03:43:45PM -0600, Shuah Khan wrote: > On 4/16/21 2:53 PM, Tom Seewald wrote: > > From: Shuah Khan <skhan@linuxfoundation.org> > > > > commit 4e9c93af7279b059faf5bb1897ee90512b258a12 upstream. > > > > Fuzzing uncovered race condition between sysfs code paths in usbip > > drivers. Device connect/disconnect code paths initiated through > > sysfs interface are prone to races if disconnect happens during > > connect and vice versa. > > > > This problem is common to all drivers while it can be reproduced easily > > in vhci_hcd. Add a sysfs_lock to usbip_device struct to protect the paths. > > > > Use this in vhci_hcd to protect sysfs paths. For a complete fix, usip_host > > and usip-vudc drivers and the event handler will have to use this lock to > > protect the paths. These changes will be done in subsequent patches. > > > > Cc: stable@vger.kernel.org # 4.9.x > > Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > > Link: https://lore.kernel.org/r/b6568f7beae702bbc236a545d3c020106ca75eac.1616807117.git.skhan@linuxfoundation.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > --- > > drivers/usb/usbip/usbip_common.h | 3 +++ > > drivers/usb/usbip/vhci_hcd.c | 1 + > > drivers/usb/usbip/vhci_sysfs.c | 30 +++++++++++++++++++++++++----- > > 3 files changed, 29 insertions(+), 5 deletions(-) > > > > Thank you for the backport. > > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> > > Greg, please pick this up for 4.9.x Also for 4.14.y, right?
On 4/19/21 6:23 AM, Greg Kroah-Hartman wrote: > On Fri, Apr 16, 2021 at 03:43:45PM -0600, Shuah Khan wrote: >> On 4/16/21 2:53 PM, Tom Seewald wrote: >>> From: Shuah Khan <skhan@linuxfoundation.org> >>> >>> commit 4e9c93af7279b059faf5bb1897ee90512b258a12 upstream. >>> >>> Fuzzing uncovered race condition between sysfs code paths in usbip >>> drivers. Device connect/disconnect code paths initiated through >>> sysfs interface are prone to races if disconnect happens during >>> connect and vice versa. >>> >>> This problem is common to all drivers while it can be reproduced easily >>> in vhci_hcd. Add a sysfs_lock to usbip_device struct to protect the paths. >>> >>> Use this in vhci_hcd to protect sysfs paths. For a complete fix, usip_host >>> and usip-vudc drivers and the event handler will have to use this lock to >>> protect the paths. These changes will be done in subsequent patches. >>> >>> Cc: stable@vger.kernel.org # 4.9.x >>> Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com >>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >>> Link: https://lore.kernel.org/r/b6568f7beae702bbc236a545d3c020106ca75eac.1616807117.git.skhan@linuxfoundation.org >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> Signed-off-by: Tom Seewald <tseewald@gmail.com> >>> --- >>> drivers/usb/usbip/usbip_common.h | 3 +++ >>> drivers/usb/usbip/vhci_hcd.c | 1 + >>> drivers/usb/usbip/vhci_sysfs.c | 30 +++++++++++++++++++++++++----- >>> 3 files changed, 29 insertions(+), 5 deletions(-) >>> >> >> Thank you for the backport. >> >> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> >> >> Greg, please pick this up for 4.9.x > > Also for 4.14.y, right? > It made it into 4.14 already. We are good with 4.14.y 5f2a149564ee2b41ab09e90add21153bd5be64d3 thanks, -- Shuah
On Mon, Apr 19, 2021 at 03:42:06PM -0600, Shuah Khan wrote: > On 4/19/21 6:23 AM, Greg Kroah-Hartman wrote: > > On Fri, Apr 16, 2021 at 03:43:45PM -0600, Shuah Khan wrote: > > > On 4/16/21 2:53 PM, Tom Seewald wrote: > > > > From: Shuah Khan <skhan@linuxfoundation.org> > > > > > > > > commit 4e9c93af7279b059faf5bb1897ee90512b258a12 upstream. > > > > > > > > Fuzzing uncovered race condition between sysfs code paths in usbip > > > > drivers. Device connect/disconnect code paths initiated through > > > > sysfs interface are prone to races if disconnect happens during > > > > connect and vice versa. > > > > > > > > This problem is common to all drivers while it can be reproduced easily > > > > in vhci_hcd. Add a sysfs_lock to usbip_device struct to protect the paths. > > > > > > > > Use this in vhci_hcd to protect sysfs paths. For a complete fix, usip_host > > > > and usip-vudc drivers and the event handler will have to use this lock to > > > > protect the paths. These changes will be done in subsequent patches. > > > > > > > > Cc: stable@vger.kernel.org # 4.9.x > > > > Reported-and-tested-by: syzbot+a93fba6d384346a761e3@syzkaller.appspotmail.com > > > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > > > > Link: https://lore.kernel.org/r/b6568f7beae702bbc236a545d3c020106ca75eac.1616807117.git.skhan@linuxfoundation.org > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > Signed-off-by: Tom Seewald <tseewald@gmail.com> > > > > --- > > > > drivers/usb/usbip/usbip_common.h | 3 +++ > > > > drivers/usb/usbip/vhci_hcd.c | 1 + > > > > drivers/usb/usbip/vhci_sysfs.c | 30 +++++++++++++++++++++++++----- > > > > 3 files changed, 29 insertions(+), 5 deletions(-) > > > > > > > > > > Thank you for the backport. > > > > > > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> > > > > > > Greg, please pick this up for 4.9.x > > > > Also for 4.14.y, right? > > > > It made it into 4.14 already. We are good with 4.14.y > > 5f2a149564ee2b41ab09e90add21153bd5be64d3 Ugh, sorry, my fault, I hadn't updated my "what was released in what stable version" on my laptop that I was working from yesterday. They are obviously all merged in 4.14 :( Thanks for this. greg k-h
diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h index 0b199a2664c0..3d47c681aea2 100644 --- a/drivers/usb/usbip/usbip_common.h +++ b/drivers/usb/usbip/usbip_common.h @@ -278,6 +278,9 @@ struct usbip_device { /* lock for status */ spinlock_t lock; + /* mutex for synchronizing sysfs store paths */ + struct mutex sysfs_lock; + int sockfd; struct socket *tcp_socket; diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 8bda6455dfcb..fb7b03029b8e 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -907,6 +907,7 @@ static void vhci_device_init(struct vhci_device *vdev) vdev->ud.side = USBIP_VHCI; vdev->ud.status = VDEV_ST_NULL; spin_lock_init(&vdev->ud.lock); + mutex_init(&vdev->ud.sysfs_lock); INIT_LIST_HEAD(&vdev->priv_rx); INIT_LIST_HEAD(&vdev->priv_tx); diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c index ca00d38d22af..3496b402aa1b 100644 --- a/drivers/usb/usbip/vhci_sysfs.c +++ b/drivers/usb/usbip/vhci_sysfs.c @@ -161,6 +161,8 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci, __u32 rhport) usbip_dbg_vhci_sysfs("enter\n"); + mutex_lock(&vdev->ud.sysfs_lock); + /* lock */ spin_lock_irqsave(&vhci->lock, flags); spin_lock(&vdev->ud.lock); @@ -171,6 +173,7 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci, __u32 rhport) /* unlock */ spin_unlock(&vdev->ud.lock); spin_unlock_irqrestore(&vhci->lock, flags); + mutex_unlock(&vdev->ud.sysfs_lock); return -EINVAL; } @@ -181,6 +184,8 @@ static int vhci_port_disconnect(struct vhci_hcd *vhci, __u32 rhport) usbip_event_add(&vdev->ud, VDEV_EVENT_DOWN); + mutex_unlock(&vdev->ud.sysfs_lock); + return 0; } @@ -309,30 +314,36 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, vhci = hcd_to_vhci(hcd); vdev = &vhci->vdev[rhport]; + mutex_lock(&vdev->ud.sysfs_lock); + /* Extract socket from fd. */ socket = sockfd_lookup(sockfd, &err); if (!socket) { dev_err(dev, "failed to lookup sock"); - return -EINVAL; + err = -EINVAL; + goto unlock_mutex; } if (socket->type != SOCK_STREAM) { dev_err(dev, "Expecting SOCK_STREAM - found %d", socket->type); sockfd_put(socket); - return -EINVAL; + err = -EINVAL; + goto unlock_mutex; } /* create threads before locking */ tcp_rx = kthread_create(vhci_rx_loop, &vdev->ud, "vhci_rx"); if (IS_ERR(tcp_rx)) { sockfd_put(socket); - return -EINVAL; + err = -EINVAL; + goto unlock_mutex; } tcp_tx = kthread_create(vhci_tx_loop, &vdev->ud, "vhci_tx"); if (IS_ERR(tcp_tx)) { kthread_stop(tcp_rx); sockfd_put(socket); - return -EINVAL; + err = -EINVAL; + goto unlock_mutex; } /* get task structs now */ @@ -353,7 +364,8 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, kthread_stop_put(tcp_tx); dev_err(dev, "port %d already used\n", rhport); - return -EINVAL; + err = -EINVAL; + goto unlock_mutex; } dev_info(dev, "pdev(%u) rhport(%u) sockfd(%d)\n", @@ -378,7 +390,15 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr, rh_port_connect(vdev, speed); + dev_info(dev, "Device attached\n"); + + mutex_unlock(&vdev->ud.sysfs_lock); + return count; + +unlock_mutex: + mutex_unlock(&vdev->ud.sysfs_lock); + return err; } static DEVICE_ATTR(attach, S_IWUSR, NULL, store_attach);