| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <6ef62df7d7e421dbd71d065f7c1472a8d7c187e6.1518036867.git.shuahkh@osg.samsung.com>
Date: Wed, 7 Feb 2018 14:20:26 -0700
From: Shuah Khan <shuahkh@....samsung.com>
To: valentina.manea.m@...il.com, shuah@...nel.org,
gregkh@...uxfoundation.org
Cc: Shuah Khan <shuahkh@....samsung.com>, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: [PATCH 3.18 3/9] usbip: prevent vhci_hcd driver from leaking a socket pointer address
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: Secunia Research <vuln@...unia.com>
Signed-off-by: Shuah Khan <shuahkh@....samsung.com>
---
drivers/usb/usbip/usbip_common.h | 1 +
drivers/usb/usbip/vhci_sysfs.c | 26 +++++++++++++++-----------
tools/usb/usbip/libsrc/vhci_driver.c | 8 ++++----
3 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/usbip/usbip_common.h b/drivers/usb/usbip/usbip_common.h
index 86b08475c254..f875ccaa55f9 100644
--- a/drivers/usb/usbip/usbip_common.h
+++ b/drivers/usb/usbip/usbip_common.h
@@ -261,6 +261,7 @@ struct usbip_device {
/* lock for status */
spinlock_t lock;
+ int sockfd;
struct socket *tcp_socket;
struct task_struct *tcp_rx;
diff --git a/drivers/usb/usbip/vhci_sysfs.c b/drivers/usb/usbip/vhci_sysfs.c
index 211f43f67ea2..f05f1e0a2baf 100644
--- a/drivers/usb/usbip/vhci_sysfs.c
+++ b/drivers/usb/usbip/vhci_sysfs.c
@@ -39,16 +39,20 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
/*
* output example:
- * prt sta spd dev socket local_busid
- * 000 004 000 000 c5a7bb80 1-2.3
- * 001 004 000 000 d8cee980 2-3.4
+ * prt sta spd dev sockfd local_busid
+ * 0000 004 000 00000000 000003 1-2.3
+ * 0001 004 000 00000000 000004 2-3.4
*
- * IP address can be retrieved from a socket pointer address by looking
- * up /proc/net/{tcp,tcp6}. Also, a userland program may remember a
- * port number and its peer IP address.
+ * Output includes socket fd instead of socket pointer address to
+ * avoid leaking kernel memory address in:
+ * /sys/devices/platform/vhci_hcd.0/status and in debug output.
+ * 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 change is made to use sockfd instead.
*/
out += sprintf(out,
- "prt sta spd bus dev socket 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);
@@ -59,12 +63,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, "%s", dev_name(&vdev->udev->dev));
- } else {
- out += sprintf(out, "000 000 000 0000000000000000 0-0");
- }
+ } else
+ out += sprintf(out, "000 00000000 000000 0-0");
out += sprintf(out, "\n");
spin_unlock(&vdev->ud.lock);
@@ -223,6 +226,7 @@ static ssize_t store_attach(struct device *dev, struct device_attribute *attr,
vdev->devid = devid;
vdev->speed = speed;
+ vdev->ud.sockfd = sockfd;
vdev->ud.tcp_socket = socket;
vdev->ud.status = VDEV_ST_NOTASSIGNED;
diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c
index ad9204773533..1274f326242c 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -55,12 +55,12 @@ static int parse_status(const char *value)
while (*c != '\0') {
int port, status, speed, devid;
- unsigned long socket;
+ int sockfd;
char lbusid[SYSFS_BUS_ID_SIZE];
- ret = sscanf(c, "%d %d %d %x %lx %31s\n",
+ ret = sscanf(c, "%d %d %d %x %u %31s\n",
&port, &status, &speed,
- &devid, &socket, lbusid);
+ &devid, &sockfd, lbusid);
if (ret < 5) {
dbg("sscanf failed: %d", ret);
@@ -69,7 +69,7 @@ static int parse_status(const char *value)
dbg("port %d status %d speed %d devid %x",
port, status, speed, devid);
- dbg("socket %lx lbusid %s", socket, lbusid);
+ dbg("sockfd %u lbusid %s", sockfd, lbusid);
/* if a device is connected, look at it */
--
2.14.1
Powered by blists - more mailing lists