Revert "xhci: Fix memory leak when caching protocol extended capability PSI tables"

This reverts commit fc57313d1017dd6b6f37a94e88daa8df54368ecc.

Marek reports that it breaks things:
	This patch landed in today's linux-next (20200211) and causes
	NULL pointer dereference during second suspend/resume cycle on
	Samsung Exynos5422-based (arm 32bit) Odroid XU3lite board:

A more complete fix will be added soon.

Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Fixes: fc57313d1017 ("xhci: Fix memory leak when caching protocol extended capability PSI tables")
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
Cc: Sajja Venkateswara Rao <VenkateswaraRao.Sajja@amd.com>
Cc: stable <stable@vger.kernel.org> # v4.4+
Cc: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index af92b2576fe91c5d497ab0aa8145aeaabd9ab5c9..7a3a29e5e9d29d33bec4e9ff45e50f26986dbf96 100644 (file)
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -55,7 +55,6 @@ static u8 usb_bos_descriptor [] = {
 static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
                                     u16 wLength)
 {
-       struct xhci_port_cap *port_cap = NULL;
        int i, ssa_count;
        u32 temp;
        u16 desc_size, ssp_cap_size, ssa_size = 0;
@@ -65,24 +64,16 @@ static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
        ssp_cap_size = sizeof(usb_bos_descriptor) - desc_size;
 
        /* does xhci support USB 3.1 Enhanced SuperSpeed */
-       for (i = 0; i < xhci->num_port_caps; i++) {
-               if (xhci->port_caps[i].maj_rev == 0x03 &&
-                   xhci->port_caps[i].min_rev >= 0x01) {
-                       usb3_1 = true;
-                       port_cap = &xhci->port_caps[i];
-                       break;
-               }
-       }
-
-       if (usb3_1) {
+       if (xhci->usb3_rhub.min_rev >= 0x01) {
                /* does xhci provide a PSI table for SSA speed attributes? */
-               if (port_cap->psi_count) {
+               if (xhci->usb3_rhub.psi_count) {
                        /* two SSA entries for each unique PSI ID, RX and TX */
-                       ssa_count = port_cap->psi_uid_count * 2;
+                       ssa_count = xhci->usb3_rhub.psi_uid_count * 2;
                        ssa_size = ssa_count * sizeof(u32);
                        ssp_cap_size -= 16; /* skip copying the default SSA */
                }
                desc_size += ssp_cap_size;
+               usb3_1 = true;
        }
        memcpy(buf, &usb_bos_descriptor, min(desc_size, wLength));
 
@@ -108,7 +99,7 @@ static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
        }
 
        /* If PSI table exists, add the custom speed attributes from it */
-       if (usb3_1 && port_cap->psi_count) {
+       if (usb3_1 && xhci->usb3_rhub.psi_count) {
                u32 ssp_cap_base, bm_attrib, psi, psi_mant, psi_exp;
                int offset;
 
@@ -120,7 +111,7 @@ static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
 
                /* attribute count SSAC bits 4:0 and ID count SSIC bits 8:5 */
                bm_attrib = (ssa_count - 1) & 0x1f;
-               bm_attrib |= (port_cap->psi_uid_count - 1) << 5;
+               bm_attrib |= (xhci->usb3_rhub.psi_uid_count - 1) << 5;
                put_unaligned_le32(bm_attrib, &buf[ssp_cap_base + 4]);
 
                if (wLength < desc_size + ssa_size)
@@ -133,8 +124,8 @@ static int xhci_create_usb3_bos_desc(struct xhci_hcd *xhci, char *buf,
                 * USB 3.1 requires two SSA entries (RX and TX) for every link
                 */
                offset = desc_size;
-               for (i = 0; i < port_cap->psi_count; i++) {
-                       psi = port_cap->psi[i];
+               for (i = 0; i < xhci->usb3_rhub.psi_count; i++) {
+                       psi = xhci->usb3_rhub.psi[i];
                        psi &= ~USB_SSP_SUBLINK_SPEED_RSVD;
                        psi_exp = XHCI_EXT_PORT_PSIE(psi);
                        psi_mant = XHCI_EXT_PORT_PSIM(psi);

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index bd5b152df6c0d87ffa1ba608f2ca0ccc1674b832..0e2701649369a18fadeef8ffd5023c766714d017 100644 (file)
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1915,16 +1915,17 @@ no_bw:
        xhci->usb3_rhub.num_ports = 0;
        xhci->num_active_eps = 0;
        kfree(xhci->usb2_rhub.ports);
+       kfree(xhci->usb2_rhub.psi);
        kfree(xhci->usb3_rhub.ports);
+       kfree(xhci->usb3_rhub.psi);
        kfree(xhci->hw_ports);
        kfree(xhci->rh_bw);
        kfree(xhci->ext_caps);
-       for (i = 0; i < xhci->num_port_caps; i++)
-               kfree(xhci->port_caps[i].psi);
-       kfree(xhci->port_caps);
 
        xhci->usb2_rhub.ports = NULL;
+       xhci->usb2_rhub.psi = NULL;
        xhci->usb3_rhub.ports = NULL;
+       xhci->usb3_rhub.psi = NULL;
        xhci->hw_ports = NULL;
        xhci->rh_bw = NULL;
        xhci->ext_caps = NULL;
@@ -2125,7 +2126,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
        u8 major_revision, minor_revision;
        struct xhci_hub *rhub;
        struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
-       struct xhci_port_cap *port_cap;
 
        temp = readl(addr);
        major_revision = XHCI_EXT_PORT_MAJOR(temp);
@@ -2160,39 +2160,31 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
                /* WTF? "Valid values are ‘1’ to MaxPorts" */
                return;
 
-       port_cap = &xhci->port_caps[xhci->num_port_caps++];
-       if (xhci->num_port_caps > max_caps)
-               return;
-
-       port_cap->maj_rev = major_revision;
-       port_cap->min_rev = minor_revision;
-       port_cap->psi_count = XHCI_EXT_PORT_PSIC(temp);
-
-       if (port_cap->psi_count) {
-               port_cap->psi = kcalloc_node(port_cap->psi_count,
-                                            sizeof(*port_cap->psi),
-                                            GFP_KERNEL, dev_to_node(dev));
-               if (!port_cap->psi)
-                       port_cap->psi_count = 0;
+       rhub->psi_count = XHCI_EXT_PORT_PSIC(temp);
+       if (rhub->psi_count) {
+               rhub->psi = kcalloc_node(rhub->psi_count, sizeof(*rhub->psi),
+                                   GFP_KERNEL, dev_to_node(dev));
+               if (!rhub->psi)
+                       rhub->psi_count = 0;
 
-               port_cap->psi_uid_count++;
-               for (i = 0; i < port_cap->psi_count; i++) {
-                       port_cap->psi[i] = readl(addr + 4 + i);
+               rhub->psi_uid_count++;
+               for (i = 0; i < rhub->psi_count; i++) {
+                       rhub->psi[i] = readl(addr + 4 + i);
 
                        /* count unique ID values, two consecutive entries can
                         * have the same ID if link is assymetric
                         */
-                       if (i && (XHCI_EXT_PORT_PSIV(port_cap->psi[i]) !=
-                                 XHCI_EXT_PORT_PSIV(port_cap->psi[i - 1])))
-                               port_cap->psi_uid_count++;
+                       if (i && (XHCI_EXT_PORT_PSIV(rhub->psi[i]) !=
+                                 XHCI_EXT_PORT_PSIV(rhub->psi[i - 1])))
+                               rhub->psi_uid_count++;
 
                        xhci_dbg(xhci, "PSIV:%d PSIE:%d PLT:%d PFD:%d LP:%d PSIM:%d\n",
-                                 XHCI_EXT_PORT_PSIV(port_cap->psi[i]),
-                                 XHCI_EXT_PORT_PSIE(port_cap->psi[i]),
-                                 XHCI_EXT_PORT_PLT(port_cap->psi[i]),
-                                 XHCI_EXT_PORT_PFD(port_cap->psi[i]),
-                                 XHCI_EXT_PORT_LP(port_cap->psi[i]),
-                                 XHCI_EXT_PORT_PSIM(port_cap->psi[i]));
+                                 XHCI_EXT_PORT_PSIV(rhub->psi[i]),
+                                 XHCI_EXT_PORT_PSIE(rhub->psi[i]),
+                                 XHCI_EXT_PORT_PLT(rhub->psi[i]),
+                                 XHCI_EXT_PORT_PFD(rhub->psi[i]),
+                                 XHCI_EXT_PORT_LP(rhub->psi[i]),
+                                 XHCI_EXT_PORT_PSIM(rhub->psi[i]));
                }
        }
        /* cache usb2 port capabilities */
@@ -2227,7 +2219,6 @@ static void xhci_add_in_port(struct xhci_hcd *xhci, unsigned int num_ports,
                        continue;
                }
                hw_port->rhub = rhub;
-               hw_port->port_cap = port_cap;
                rhub->num_ports++;
        }
        /* FIXME: Should we disable ports not in the Extended Capabilities? */
@@ -2318,11 +2309,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd *xhci, gfp_t flags)
        if (!xhci->ext_caps)
                return -ENOMEM;
 
-       xhci->port_caps = kcalloc_node(cap_count, sizeof(*xhci->port_caps),
-                               flags, dev_to_node(dev));
-       if (!xhci->port_caps)
-               return -ENOMEM;
-
        offset = cap_start;
 
        while (offset) {

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 3ecee10fdcdc7dfd697b24310de0ad3885421f10..13d8838cd552be01b145e0d2f78dc815c3ffad0b 100644 (file)
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1702,20 +1702,12 @@ struct xhci_bus_state {
  * Intel Lynx Point LP xHCI host.
  */
 #define        XHCI_MAX_REXIT_TIMEOUT_MS       20
-struct xhci_port_cap {
-       u32                     *psi;   /* array of protocol speed ID entries */
-       u8                      psi_count;
-       u8                      psi_uid_count;
-       u8                      maj_rev;
-       u8                      min_rev;
-};
 
 struct xhci_port {
        __le32 __iomem          *addr;
        int                     hw_portnum;
        int                     hcd_portnum;
        struct xhci_hub         *rhub;
-       struct xhci_port_cap    *port_cap;
 };
 
 struct xhci_hub {
@@ -1727,6 +1719,9 @@ struct xhci_hub {
        /* supported prococol extended capabiliy values */
        u8                      maj_rev;
        u8                      min_rev;
+       u32                     *psi;   /* array of protocol speed ID entries */
+       u8                      psi_count;
+       u8                      psi_uid_count;
 };
 
 /* There is one xhci_hcd structure per controller */
@@ -1885,9 +1880,6 @@ struct xhci_hcd {
        /* cached usb2 extened protocol capabilites */
        u32                     *ext_caps;
        unsigned int            num_ext_caps;
-       /* cached extended protocol port capabilities */
-       struct xhci_port_cap    *port_caps;
-       unsigned int            num_port_caps;
        /* Compliance Mode Recovery Data */
        struct timer_list       comp_mode_recovery_timer;
        u32                     port_status_u0;