2018-02-05 17:47:07

by Shuah Khan

[permalink] [raw]
Subject: [PATCH 4.4 v2 0/2] Backports for fixes

The first patch in this series isolates and backports a fix to clear
just the USB_PORT_STAT_POWER. Without this fix, client can't use the
imported device.

The second patch is fix to back-ported commit 3eee23c3ec14. tcp_socket
address still present in the status file. This is my bad. The bug fixed
in the first patch in this series masked this bug. With these two fixes,
client can use the imported devices on 4.4

Eric Biggers also reported the tcp_socket address still in the status
file while I am getting the patch ready. I added him to Reported-by.

Shuah Khan (2):
usbip: vhci_hcd: clear just the USB_PORT_STAT_POWER bit
usbip: fix 3eee23c3ec14 tcp_socket address still in the status file

drivers/usb/usbip/vhci_hcd.c | 2 +-
drivers/usb/usbip/vhci_sysfs.c | 7 +++----
2 files changed, 4 insertions(+), 5 deletions(-)

--
2.14.1



2018-02-05 17:47:26

by Shuah Khan

[permalink] [raw]
Subject: [PATCH 4.4 v2 1/2] usbip: vhci_hcd: clear just the USB_PORT_STAT_POWER bit

Upstream commit 1c9de5bf4286 ("usbip: vhci-hcd: Add USB3 SuperSpeed
support")

vhci_hcd clears all the bits port_status bits instead of clearing
just the USB_PORT_STAT_POWER bit when it handles ClearPortFeature:
USB_PORT_FEAT_POWER. This causes vhci_hcd attach to fail in a bad
state, leaving device unusable by the client. The device is still
attached and however client can't use it.

The problem was fixed as part of larger change to add USB3 Super Speed
support.

This patch isolates the one line fix to clear the USB_PORT_STAT_POWER
from the original patch.

Signed-off-by: Shuah Khan <[email protected]>
---
drivers/usb/usbip/vhci_hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 00d68945548e..2d96bfd34138 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -285,7 +285,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
case USB_PORT_FEAT_POWER:
usbip_dbg_vhci_rh(
" ClearPortFeature: USB_PORT_FEAT_POWER\n");
- dum->port_status[rhport] = 0;
+ dum->port_status[rhport] &= ~USB_PORT_STAT_POWER;
dum->resuming = 0;
break;
case USB_PORT_FEAT_C_RESET:
--
2.14.1


2018-02-05 17:49:31

by Shuah Khan

[permalink] [raw]
Subject: [PATCH 4.4 v2 2/2] usbip: fix 3eee23c3ec14 tcp_socket address still in the status file

Commit 3eee23c3ec14 ("usbip: prevent vhci_hcd driver from leaking a
socket pointer address") backported the following commit from mailine.
However, backport error caused the tcp_socket address to still leak.

commit 2f2d0088eb93 ("usbip: prevent vhci_hcd driver from leaking a
socket pointer address")

When a client has a USB device attached over IP, the vhci_hcd driver is
locally leaking a socket pointer address via the

/sys/devices/platform/vhci_hcd/status file (world-readable) and in debug
output when "usbip --debug port" is run.

Fix it to not leak. The socket pointer address is not used at the moment
and it was made visible as a convenient way to find IP address from
socket pointer address by looking up /proc/net/{tcp,tcp6}.

As this opens a security hole, the fix replaces socket pointer address
with sockfd.

Reported-by: Eric Biggers <[email protected]>
Signed-off-by: Shuah Khan <[email protected]>
---
drivers/usb/usbip/vhci_sysfs.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 1c7f41a65565..b9432fdec775 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -53,7 +53,7 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
* a security hole, the change is made to use sockfd instead.
*/
out += sprintf(out,
- "prt sta spd bus dev sockfd local_busid\n");
+ "prt sta spd dev sockfd local_busid\n");

for (i = 0; i < VHCI_NPORTS; i++) {
struct vhci_device *vdev = port_to_vdev(i);
@@ -64,12 +64,11 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
if (vdev->ud.status == VDEV_ST_USED) {
out += sprintf(out, "%03u %08x ",
vdev->speed, vdev->devid);
- out += sprintf(out, "%16p ", vdev->ud.tcp_socket);
- out += sprintf(out, "%06u", vdev->ud.sockfd);
+ out += sprintf(out, "%06u ", vdev->ud.sockfd);
out += sprintf(out, "%s", dev_name(&vdev->udev->dev));

} else
- out += sprintf(out, "000 000 000 000000 0-0");
+ out += sprintf(out, "000 00000000 000000 0-0");

out += sprintf(out, "\n");
spin_unlock(&vdev->ud.lock);
--
2.14.1