Re: [bug report] PCI: Protect pci_reassign_bridge_resources() against addition/removal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2019-09-04 at 13:03 +0300, Dan Carpenter wrote:
> Hello Benjamin Herrenschmidt,
> 
> The patch 540f62d26f02: "PCI: Protect pci_reassign_bridge_resources()
> against addition/removal" from Jun 24, 2019, leads to the following
> static checker warning:
> 
> 	drivers/pci/setup-bus.c:2158 pci_reassign_bridge_resources()
> 	warn: inconsistent returns 'read_sem:&pci_bus_sem'.

Thanks, I missed that. I'll try to send something but from Plumbers
might be difficult...

Cheers
Ben.

> drivers/pci/setup-bus.c
>   2066          unsigned int i;
>   2067          int ret;
>   2068  
>   2069          down_read(&pci_bus_sem);
> 
> We added some new locking here.
> 
>   2070  
>   2071          /* Walk to the root hub, releasing bridge BARs when
> possible */
>   2072          next = bridge;
>   2073          do {
>   2074                  bridge = next;
>   2075                  for (i = PCI_BRIDGE_RESOURCES; i <
> PCI_BRIDGE_RESOURCE_END;
>   2076                       i++) {
>   2077                          struct resource *res = &bridge-
> >resource[i];
>   2078  
>   2079                          if ((res->flags ^ type) &
> PCI_RES_TYPE_MASK)
>   2080                                  continue;
>   2081  
>   2082                          /* Ignore BARs which are still in use
> */
>   2083                          if (res->child)
>   2084                                  continue;
>   2085  
>   2086                          ret = add_to_list(&saved, bridge,
> res, 0, 0);
>   2087                          if (ret)
>   2088                                  goto cleanup;
>   2089  
>   2090                          pci_info(bridge, "BAR %d: releasing
> %pR\n",
>   2091                                   i, res);
>   2092  
>   2093                          if (res->parent)
>   2094                                  release_resource(res);
>   2095                          res->start = 0;
>   2096                          res->end = 0;
>   2097                          break;
>   2098                  }
>   2099                  if (i == PCI_BRIDGE_RESOURCE_END)
>   2100                          break;
>   2101  
>   2102                  next = bridge->bus ? bridge->bus->self :
> NULL;
>   2103          } while (next);
>   2104  
>   2105          if (list_empty(&saved))
>   2106                  return -ENOENT;
> 
> This needs an unlock.  It's not clear to me if any other clean up is
> required, but possibly it's worth looking at.
> 
>   2107  
>   2108          __pci_bus_size_bridges(bridge->subordinate, &added);
>   2109          __pci_bridge_assign_resources(bridge, &added,
> &failed);
> 
> regards,
> dan carpenter




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux