* [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() [not found] <20210727131908.834086-1-sashal@kernel.org> @ 2021-07-27 13:18 ` Sasha Levin 2021-07-27 13:28 ` Greg Kroah-Hartman 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 07/21] r8152: Fix potential PM refcount imbalance Sasha Levin 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 08/21] r8152: Fix a deadlock by doubly PM resume Sasha Levin 2 siblings, 1 reply; 5+ messages in thread From: Sasha Levin @ 2021-07-27 13:18 UTC (permalink / raw) To: linux-kernel, stable Cc: Alan Stern, syzbot+72af3105289dcb4c055b, Greg Kroah-Hartman, Sasha Levin, linux-usb From: Alan Stern <stern@rowland.harvard.edu> [ Upstream commit b0863f1927323110e3d0d69f6adb6a91018a9a3c ] When the user submits a control URB via usbfs, the user supplies the bRequestType value and the kernel uses it to compute the pipe value. However, do_proc_control() performs this computation incorrectly in the case where the bRequestType direction bit is set to USB_DIR_IN and the URB's transfer length is 0: The pipe's direction is also set to IN but it should be OUT, which is the direction the actual transfer will use regardless of bRequestType. Commit 5cc59c418fde ("USB: core: WARN if pipe direction != setup packet direction") added a check to compare the direction bit in the pipe value to a control URB's actual direction and to WARN if they are different. This can be triggered by the incorrect computation mentioned above, as found by syzbot. This patch fixes the computation, thus avoiding the WARNing. Reported-and-tested-by: syzbot+72af3105289dcb4c055b@syzkaller.appspotmail.com Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Link: https://lore.kernel.org/r/20210712185436.GB326369@rowland.harvard.edu Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/usb/core/devio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 2218941d35a3..73b60f013b20 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1133,7 +1133,7 @@ static int do_proc_control(struct usb_dev_state *ps, "wIndex=%04x wLength=%04x\n", ctrl->bRequestType, ctrl->bRequest, ctrl->wValue, ctrl->wIndex, ctrl->wLength); - if (ctrl->bRequestType & 0x80) { + if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength) { pipe = usb_rcvctrlpipe(dev, 0); snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0); -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() Sasha Levin @ 2021-07-27 13:28 ` Greg Kroah-Hartman 2021-07-27 18:54 ` Sasha Levin 0 siblings, 1 reply; 5+ messages in thread From: Greg Kroah-Hartman @ 2021-07-27 13:28 UTC (permalink / raw) To: Sasha Levin Cc: linux-kernel, stable, Alan Stern, syzbot+72af3105289dcb4c055b, linux-usb On Tue, Jul 27, 2021 at 09:18:53AM -0400, Sasha Levin wrote: > From: Alan Stern <stern@rowland.harvard.edu> > > [ Upstream commit b0863f1927323110e3d0d69f6adb6a91018a9a3c ] > > When the user submits a control URB via usbfs, the user supplies the > bRequestType value and the kernel uses it to compute the pipe value. > However, do_proc_control() performs this computation incorrectly in > the case where the bRequestType direction bit is set to USB_DIR_IN and > the URB's transfer length is 0: The pipe's direction is also set to IN > but it should be OUT, which is the direction the actual transfer will > use regardless of bRequestType. > > Commit 5cc59c418fde ("USB: core: WARN if pipe direction != setup > packet direction") added a check to compare the direction bit in the > pipe value to a control URB's actual direction and to WARN if they are > different. This can be triggered by the incorrect computation > mentioned above, as found by syzbot. > > This patch fixes the computation, thus avoiding the WARNing. > > Reported-and-tested-by: syzbot+72af3105289dcb4c055b@syzkaller.appspotmail.com > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Link: https://lore.kernel.org/r/20210712185436.GB326369@rowland.harvard.edu > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Sasha Levin <sashal@kernel.org> > --- > drivers/usb/core/devio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 2218941d35a3..73b60f013b20 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -1133,7 +1133,7 @@ static int do_proc_control(struct usb_dev_state *ps, > "wIndex=%04x wLength=%04x\n", > ctrl->bRequestType, ctrl->bRequest, ctrl->wValue, > ctrl->wIndex, ctrl->wLength); > - if (ctrl->bRequestType & 0x80) { > + if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength) { > pipe = usb_rcvctrlpipe(dev, 0); > snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0); > > -- > 2.30.2 > This is not needed in any kernel that does not also have 5cc59c418fde ("USB: core: WARN if pipe direction != setup packet direction"), which showed up in 5.14-rc1, so please drop this from all of the AUTOSEL trees. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() 2021-07-27 13:28 ` Greg Kroah-Hartman @ 2021-07-27 18:54 ` Sasha Levin 0 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2021-07-27 18:54 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, stable, Alan Stern, syzbot+72af3105289dcb4c055b, linux-usb On Tue, Jul 27, 2021 at 03:28:20PM +0200, Greg Kroah-Hartman wrote: >On Tue, Jul 27, 2021 at 09:18:53AM -0400, Sasha Levin wrote: >> From: Alan Stern <stern@rowland.harvard.edu> >> >> [ Upstream commit b0863f1927323110e3d0d69f6adb6a91018a9a3c ] >> >> When the user submits a control URB via usbfs, the user supplies the >> bRequestType value and the kernel uses it to compute the pipe value. >> However, do_proc_control() performs this computation incorrectly in >> the case where the bRequestType direction bit is set to USB_DIR_IN and >> the URB's transfer length is 0: The pipe's direction is also set to IN >> but it should be OUT, which is the direction the actual transfer will >> use regardless of bRequestType. >> >> Commit 5cc59c418fde ("USB: core: WARN if pipe direction != setup >> packet direction") added a check to compare the direction bit in the >> pipe value to a control URB's actual direction and to WARN if they are >> different. This can be triggered by the incorrect computation >> mentioned above, as found by syzbot. >> >> This patch fixes the computation, thus avoiding the WARNing. >> >> Reported-and-tested-by: syzbot+72af3105289dcb4c055b@syzkaller.appspotmail.com >> Signed-off-by: Alan Stern <stern@rowland.harvard.edu> >> Link: https://lore.kernel.org/r/20210712185436.GB326369@rowland.harvard.edu >> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Signed-off-by: Sasha Levin <sashal@kernel.org> >> --- >> drivers/usb/core/devio.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c >> index 2218941d35a3..73b60f013b20 100644 >> --- a/drivers/usb/core/devio.c >> +++ b/drivers/usb/core/devio.c >> @@ -1133,7 +1133,7 @@ static int do_proc_control(struct usb_dev_state *ps, >> "wIndex=%04x wLength=%04x\n", >> ctrl->bRequestType, ctrl->bRequest, ctrl->wValue, >> ctrl->wIndex, ctrl->wLength); >> - if (ctrl->bRequestType & 0x80) { >> + if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength) { >> pipe = usb_rcvctrlpipe(dev, 0); >> snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0); >> >> -- >> 2.30.2 >> > >This is not needed in any kernel that does not also have 5cc59c418fde >("USB: core: WARN if pipe direction != setup packet direction"), which >showed up in 5.14-rc1, so please drop this from all of the AUTOSEL >trees. Will do, thanks! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH AUTOSEL 5.13 07/21] r8152: Fix potential PM refcount imbalance [not found] <20210727131908.834086-1-sashal@kernel.org> 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() Sasha Levin @ 2021-07-27 13:18 ` Sasha Levin 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 08/21] r8152: Fix a deadlock by doubly PM resume Sasha Levin 2 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2021-07-27 13:18 UTC (permalink / raw) To: linux-kernel, stable Cc: Takashi Iwai, David S . Miller, Sasha Levin, linux-usb, netdev From: Takashi Iwai <tiwai@suse.de> [ Upstream commit 9c23aa51477a37f8b56c3c40192248db0663c196 ] rtl8152_close() takes the refcount via usb_autopm_get_interface() but it doesn't release when RTL8152_UNPLUG test hits. This may lead to the imbalance of PM refcount. This patch addresses it. Link: https://bugzilla.suse.com/show_bug.cgi?id=1186194 Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/net/usb/r8152.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index e25bfb7021ed..8dcc55e4a5bc 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -6761,9 +6761,10 @@ static int rtl8152_close(struct net_device *netdev) tp->rtl_ops.down(tp); mutex_unlock(&tp->control); + } + if (!res) usb_autopm_put_interface(tp->intf); - } free_all_mem(tp); -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH AUTOSEL 5.13 08/21] r8152: Fix a deadlock by doubly PM resume [not found] <20210727131908.834086-1-sashal@kernel.org> 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() Sasha Levin 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 07/21] r8152: Fix potential PM refcount imbalance Sasha Levin @ 2021-07-27 13:18 ` Sasha Levin 2 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2021-07-27 13:18 UTC (permalink / raw) To: linux-kernel, stable Cc: Takashi Iwai, David S . Miller, Sasha Levin, linux-usb, netdev From: Takashi Iwai <tiwai@suse.de> [ Upstream commit 776ac63a986d211286230c4fd70f85390eabedcd ] r8152 driver sets up the MAC address at reset-resume, while rtl8152_set_mac_address() has the temporary autopm get/put. This may lead to a deadlock as the PM lock has been already taken for the execution of the runtime PM callback. This patch adds the workaround to avoid the superfluous autpm when called from rtl8152_reset_resume(). Link: https://bugzilla.suse.com/show_bug.cgi?id=1186194 Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/net/usb/r8152.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 8dcc55e4a5bc..2cf763b4ea84 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1550,7 +1550,8 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex, u32 advertising); -static int rtl8152_set_mac_address(struct net_device *netdev, void *p) +static int __rtl8152_set_mac_address(struct net_device *netdev, void *p, + bool in_resume) { struct r8152 *tp = netdev_priv(netdev); struct sockaddr *addr = p; @@ -1559,9 +1560,11 @@ static int rtl8152_set_mac_address(struct net_device *netdev, void *p) if (!is_valid_ether_addr(addr->sa_data)) goto out1; - ret = usb_autopm_get_interface(tp->intf); - if (ret < 0) - goto out1; + if (!in_resume) { + ret = usb_autopm_get_interface(tp->intf); + if (ret < 0) + goto out1; + } mutex_lock(&tp->control); @@ -1573,11 +1576,17 @@ static int rtl8152_set_mac_address(struct net_device *netdev, void *p) mutex_unlock(&tp->control); - usb_autopm_put_interface(tp->intf); + if (!in_resume) + usb_autopm_put_interface(tp->intf); out1: return ret; } +static int rtl8152_set_mac_address(struct net_device *netdev, void *p) +{ + return __rtl8152_set_mac_address(netdev, p, false); +} + /* Devices containing proper chips can support a persistent * host system provided MAC address. * Examples of this are Dell TB15 and Dell WD15 docks @@ -1696,7 +1705,7 @@ static int determine_ethernet_addr(struct r8152 *tp, struct sockaddr *sa) return ret; } -static int set_ethernet_addr(struct r8152 *tp) +static int set_ethernet_addr(struct r8152 *tp, bool in_resume) { struct net_device *dev = tp->netdev; struct sockaddr sa; @@ -1709,7 +1718,7 @@ static int set_ethernet_addr(struct r8152 *tp) if (tp->version == RTL_VER_01) ether_addr_copy(dev->dev_addr, sa.sa_data); else - ret = rtl8152_set_mac_address(dev, &sa); + ret = __rtl8152_set_mac_address(dev, &sa, in_resume); return ret; } @@ -8442,7 +8451,7 @@ static int rtl8152_reset_resume(struct usb_interface *intf) clear_bit(SELECTIVE_SUSPEND, &tp->flags); tp->rtl_ops.init(tp); queue_delayed_work(system_long_wq, &tp->hw_phy_work, 0); - set_ethernet_addr(tp); + set_ethernet_addr(tp, true); return rtl8152_resume(intf); } @@ -9562,7 +9571,7 @@ static int rtl8152_probe(struct usb_interface *intf, tp->rtl_fw.retry = true; #endif queue_delayed_work(system_long_wq, &tp->hw_phy_work, 0); - set_ethernet_addr(tp); + set_ethernet_addr(tp, false); usb_set_intfdata(intf, tp); -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-27 18:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210727131908.834086-1-sashal@kernel.org> 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 06/21] USB: core: Fix incorrect pipe calculation in do_proc_control() Sasha Levin 2021-07-27 13:28 ` Greg Kroah-Hartman 2021-07-27 18:54 ` Sasha Levin 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 07/21] r8152: Fix potential PM refcount imbalance Sasha Levin 2021-07-27 13:18 ` [PATCH AUTOSEL 5.13 08/21] r8152: Fix a deadlock by doubly PM resume Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).