Someone else is already working on a patch to fix part of this:

http://marc.info/?l=linux-usb&m=136509667003716&w=2

Please coordinate with them.

Also, two people trying to solve the same simple bug and using almost
exactly the same commit messages seems like too much of a coincidence.
Are you working on the same project or did you both happen to run the
same static code checking tool at the same time, or ?

Sarah Sharp

On Thu, Apr 04, 2013 at 08:31:26PM +0400, Vladimir Murzin wrote:
> Commits
>       9574323 xHCI: test USB2 software LPM
>       b92cc66 xHCI: add aborting command ring function
> introduce useful functions which involves lists manipulations.
> 
> If for whatever reason we fall into fail path in xhci_mem_init() we
> may access the lists in xhci_mem_cleanup() before they get
> initialized.
> 
> The same init-fail-cleanup case is applicable to bw tables too.
> 
> Fix this by moving list initialization code to the beginning of
> xhci_mem_init()
> 
> Reported-by: Sergey Dyasly <dse...@gmail.com>
> Tested-by: Sergey Dyasly <dse...@gmail.com>
> Signed-off-by: Vladimir Murzin <murzi...@gmail.com>
> ---
>  drivers/usb/host/xhci-mem.c |   37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 6dc238c..7b5d2f5 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -2123,7 +2123,7 @@ static int xhci_setup_port_arrays(struct xhci_hcd 
> *xhci, gfp_t flags)
>       __le32 __iomem *addr;
>       u32 offset;
>       unsigned int num_ports;
> -     int i, j, port_index;
> +     int i, port_index;
>  
>       addr = &xhci->cap_regs->hcc_params;
>       offset = XHCI_HCC_EXT_CAPS(xhci_readl(xhci, addr));
> @@ -2138,18 +2138,6 @@ static int xhci_setup_port_arrays(struct xhci_hcd 
> *xhci, gfp_t flags)
>       if (!xhci->port_array)
>               return -ENOMEM;
>  
> -     xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags);
> -     if (!xhci->rh_bw)
> -             return -ENOMEM;
> -     for (i = 0; i < num_ports; i++) {
> -             struct xhci_interval_bw_table *bw_table;
> -
> -             INIT_LIST_HEAD(&xhci->rh_bw[i].tts);
> -             bw_table = &xhci->rh_bw[i].bw_table;
> -             for (j = 0; j < XHCI_MAX_INTERVAL; j++)
> -                     INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints);
> -     }
> -
>       /*
>        * For whatever reason, the first capability offset is from the
>        * capability register base, not from the HCCPARAMS register.
> @@ -2253,7 +2241,25 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>       u64             val_64;
>       struct xhci_segment     *seg;
>       u32 page_size, temp;
> -     int i;
> +     int i, j, num_ports;
> +
> +     INIT_LIST_HEAD(&xhci->cancel_cmd_list);
> +     INIT_LIST_HEAD(&xhci->lpm_failed_devs);
> +
> +     num_ports = HCS_MAX_PORTS(xhci_readl(xhci, 
> &xhci->cap_regs->hcs_params1));
> +
> +     xhci->rh_bw = kzalloc(sizeof(*xhci->rh_bw)*num_ports, flags);
> +     if (!xhci->rh_bw)
> +             return -ENOMEM;
> +
> +     for (i = 0; i < num_ports; i++) {
> +             struct xhci_interval_bw_table *bw_table;
> +
> +             INIT_LIST_HEAD(&xhci->rh_bw[i].tts);
> +             bw_table = &xhci->rh_bw[i].bw_table;
> +             for (j = 0; j < XHCI_MAX_INTERVAL; j++)
> +                     INIT_LIST_HEAD(&bw_table->interval_bw[j].endpoints);
> +     }
>  
>       page_size = xhci_readl(xhci, &xhci->op_regs->page_size);
>       xhci_dbg(xhci, "Supported page size register = 0x%x\n", page_size);
> @@ -2333,7 +2339,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>       xhci->cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags);
>       if (!xhci->cmd_ring)
>               goto fail;
> -     INIT_LIST_HEAD(&xhci->cancel_cmd_list);
>       xhci_dbg(xhci, "Allocated command ring at %p\n", xhci->cmd_ring);
>       xhci_dbg(xhci, "First segment DMA is 0x%llx\n",
>                       (unsigned long long)xhci->cmd_ring->first_seg->dma);
> @@ -2444,8 +2449,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
>       if (xhci_setup_port_arrays(xhci, flags))
>               goto fail;
>  
> -     INIT_LIST_HEAD(&xhci->lpm_failed_devs);
> -
>       /* Enable USB 3.0 device notifications for function remote wake, which
>        * is necessary for allowing USB 3.0 devices to do remote wakeup from
>        * U3 (device suspend).
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to