* [PATCH v2 0/2] Fix find_first_zero_bit() usage @ 2017-11-17 15:00 Niklas Cassel 2017-11-17 15:00 ` [PATCH v2 1/2] PCI: designware-ep: " Niklas Cassel 2017-11-17 15:00 ` [PATCH v2 2/2] PCI: endpoint: " Niklas Cassel 0 siblings, 2 replies; 6+ messages in thread From: Niklas Cassel @ 2017-11-17 15:00 UTC (permalink / raw) To: linux-pci; +Cc: Niklas Cassel, linux-kernel From: Niklas Cassel <niklass@axis.com> find_first_zero_bit()'s parameter 'size' is defined in bits, not in bytes. Calling find_first_zero_bit() with the wrong size unit will lead to insidious bugs. Fix all uses of find_first_zero_bit() called with sizeof() as size argument in drivers/pci. Niklas Cassel (2): PCI: designware-ep: Fix find_first_zero_bit() usage PCI: endpoint: Fix find_first_zero_bit() usage drivers/pci/dwc/pcie-designware-ep.c | 6 ++---- drivers/pci/endpoint/pci-ep-cfs.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) -- 2.14.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] PCI: designware-ep: Fix find_first_zero_bit() usage 2017-11-17 15:00 [PATCH v2 0/2] Fix find_first_zero_bit() usage Niklas Cassel @ 2017-11-17 15:00 ` Niklas Cassel 2017-11-17 18:43 ` Lorenzo Pieralisi 2017-11-17 15:00 ` [PATCH v2 2/2] PCI: endpoint: " Niklas Cassel 1 sibling, 1 reply; 6+ messages in thread From: Niklas Cassel @ 2017-11-17 15:00 UTC (permalink / raw) To: Jingoo Han, Joao Pinto, Lorenzo Pieralisi, Bjorn Helgaas Cc: Niklas Cassel, linux-pci, linux-kernel find_first_zero_bit()'s parameter 'size' is defined in bits, not in bytes. find_first_zero_bit() was called with size in bytes rather than bits, which thus defined a too low upper limit, causing dw_pcie_ep_inbound_atu() to assign iatu index #4 to both bar 4 and bar 5, which made bar 5 overwrite the settings set by bar 4. Fix this by using replacing find_first_zero_bit() with ffz(), since ffz() only works on a single 'unsigned long' and therefore does not need a size argument. Fixes: f8aed6ec624f ("PCI: dwc: designware: Add EP mode support") Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> --- drivers/pci/dwc/pcie-designware-ep.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c index d53d5f168363..ab9a9e160daf 100644 --- a/drivers/pci/dwc/pcie-designware-ep.c +++ b/drivers/pci/dwc/pcie-designware-ep.c @@ -70,8 +70,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar, u32 free_win; struct dw_pcie *pci = to_dw_pcie_from_ep(ep); - free_win = find_first_zero_bit(&ep->ib_window_map, - sizeof(ep->ib_window_map)); + free_win = ffz(ep->ib_window_map); if (free_win >= ep->num_ib_windows) { dev_err(pci->dev, "no free inbound window\n"); return -EINVAL; @@ -96,8 +95,7 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t phys_addr, u32 free_win; struct dw_pcie *pci = to_dw_pcie_from_ep(ep); - free_win = find_first_zero_bit(&ep->ob_window_map, - sizeof(ep->ob_window_map)); + free_win = ffz(ep->ob_window_map); if (free_win >= ep->num_ob_windows) { dev_err(pci->dev, "no free outbound window\n"); return -EINVAL; -- 2.14.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware-ep: Fix find_first_zero_bit() usage 2017-11-17 15:00 ` [PATCH v2 1/2] PCI: designware-ep: " Niklas Cassel @ 2017-11-17 18:43 ` Lorenzo Pieralisi 2017-11-21 7:54 ` Kishon Vijay Abraham I 0 siblings, 1 reply; 6+ messages in thread From: Lorenzo Pieralisi @ 2017-11-17 18:43 UTC (permalink / raw) To: Niklas Cassel Cc: Jingoo Han, Joao Pinto, Bjorn Helgaas, Niklas Cassel, linux-pci, linux-kernel, kishon [+Kishon - please CC him next time] On Fri, Nov 17, 2017 at 04:00:40PM +0100, Niklas Cassel wrote: > find_first_zero_bit()'s parameter 'size' is defined in bits, > not in bytes. > > find_first_zero_bit() was called with size in bytes rather than bits, > which thus defined a too low upper limit, causing > dw_pcie_ep_inbound_atu() to assign iatu index #4 to both bar 4 > and bar 5, which made bar 5 overwrite the settings set by bar 4. > > Fix this by using replacing find_first_zero_bit() with ffz(), > since ffz() only works on a single 'unsigned long' and therefore > does not need a size argument. > > Fixes: f8aed6ec624f ("PCI: dwc: designware: Add EP mode support") > Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> > --- > drivers/pci/dwc/pcie-designware-ep.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c > index d53d5f168363..ab9a9e160daf 100644 > --- a/drivers/pci/dwc/pcie-designware-ep.c > +++ b/drivers/pci/dwc/pcie-designware-ep.c > @@ -70,8 +70,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar, > u32 free_win; > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > - free_win = find_first_zero_bit(&ep->ib_window_map, > - sizeof(ep->ib_window_map)); > + free_win = ffz(ep->ib_window_map); You fix the bug at hand but code suffers from the same issue since we should be checking the value against ~0UL first ie if size in bits < num_ib_windows (that I think it is unlikely anyway but while at it let's think if we can improve this). @Kishon: maybe we should add a static upper size and declare a static bitmap accordingly (and still use find_first_zero_bit()) ? I think the check against ~0UL should be added anyway, let's get Kishon's opinion before a v3 (if any). Thanks, Lorenzo > if (free_win >= ep->num_ib_windows) { > dev_err(pci->dev, "no free inbound window\n"); > return -EINVAL; > @@ -96,8 +95,7 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t phys_addr, > u32 free_win; > struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > - free_win = find_first_zero_bit(&ep->ob_window_map, > - sizeof(ep->ob_window_map)); > + free_win = ffz(ep->ob_window_map); > if (free_win >= ep->num_ob_windows) { > dev_err(pci->dev, "no free outbound window\n"); > return -EINVAL; > -- > 2.14.2 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware-ep: Fix find_first_zero_bit() usage 2017-11-17 18:43 ` Lorenzo Pieralisi @ 2017-11-21 7:54 ` Kishon Vijay Abraham I 2017-11-21 16:44 ` Lorenzo Pieralisi 0 siblings, 1 reply; 6+ messages in thread From: Kishon Vijay Abraham I @ 2017-11-21 7:54 UTC (permalink / raw) To: Lorenzo Pieralisi, Niklas Cassel Cc: Jingoo Han, Joao Pinto, Bjorn Helgaas, Niklas Cassel, linux-pci, linux-kernel Hi Lorenzo, On Saturday 18 November 2017 12:13 AM, Lorenzo Pieralisi wrote: > [+Kishon - please CC him next time] > > On Fri, Nov 17, 2017 at 04:00:40PM +0100, Niklas Cassel wrote: >> find_first_zero_bit()'s parameter 'size' is defined in bits, >> not in bytes. >> >> find_first_zero_bit() was called with size in bytes rather than bits, >> which thus defined a too low upper limit, causing >> dw_pcie_ep_inbound_atu() to assign iatu index #4 to both bar 4 >> and bar 5, which made bar 5 overwrite the settings set by bar 4. >> >> Fix this by using replacing find_first_zero_bit() with ffz(), >> since ffz() only works on a single 'unsigned long' and therefore >> does not need a size argument. >> >> Fixes: f8aed6ec624f ("PCI: dwc: designware: Add EP mode support") >> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> >> --- >> drivers/pci/dwc/pcie-designware-ep.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c >> index d53d5f168363..ab9a9e160daf 100644 >> --- a/drivers/pci/dwc/pcie-designware-ep.c >> +++ b/drivers/pci/dwc/pcie-designware-ep.c >> @@ -70,8 +70,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar, >> u32 free_win; >> struct dw_pcie *pci = to_dw_pcie_from_ep(ep); >> >> - free_win = find_first_zero_bit(&ep->ib_window_map, >> - sizeof(ep->ib_window_map)); >> + free_win = ffz(ep->ib_window_map); > > You fix the bug at hand but code suffers from the same issue since we > should be checking the value against ~0UL first ie if size in bits < > num_ib_windows (that I think it is unlikely anyway but while at it let's > think if we can improve this). > > @Kishon: maybe we should add a static upper size and declare a > static bitmap accordingly (and still use find_first_zero_bit()) ? > > I think the check against ~0UL should be added anyway, let's get > Kishon's opinion before a v3 (if any). Yeah, for both ffz and find_first_zero_bit, the results are undefined if no zero exists. I'm thinking if we should just use find_first_zero_bit(&ep->ib_window_map, ep->num_ib_windows) after checking if there are some '0' bits left. Thanks Kishon ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] PCI: designware-ep: Fix find_first_zero_bit() usage 2017-11-21 7:54 ` Kishon Vijay Abraham I @ 2017-11-21 16:44 ` Lorenzo Pieralisi 0 siblings, 0 replies; 6+ messages in thread From: Lorenzo Pieralisi @ 2017-11-21 16:44 UTC (permalink / raw) To: Kishon Vijay Abraham I Cc: Niklas Cassel, Jingoo Han, Joao Pinto, Bjorn Helgaas, Niklas Cassel, linux-pci, linux-kernel On Tue, Nov 21, 2017 at 01:24:53PM +0530, Kishon Vijay Abraham I wrote: > Hi Lorenzo, > > On Saturday 18 November 2017 12:13 AM, Lorenzo Pieralisi wrote: > > [+Kishon - please CC him next time] > > > > On Fri, Nov 17, 2017 at 04:00:40PM +0100, Niklas Cassel wrote: > >> find_first_zero_bit()'s parameter 'size' is defined in bits, > >> not in bytes. > >> > >> find_first_zero_bit() was called with size in bytes rather than bits, > >> which thus defined a too low upper limit, causing > >> dw_pcie_ep_inbound_atu() to assign iatu index #4 to both bar 4 > >> and bar 5, which made bar 5 overwrite the settings set by bar 4. > >> > >> Fix this by using replacing find_first_zero_bit() with ffz(), > >> since ffz() only works on a single 'unsigned long' and therefore > >> does not need a size argument. > >> > >> Fixes: f8aed6ec624f ("PCI: dwc: designware: Add EP mode support") > >> Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> > >> --- > >> drivers/pci/dwc/pcie-designware-ep.c | 6 ++---- > >> 1 file changed, 2 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c > >> index d53d5f168363..ab9a9e160daf 100644 > >> --- a/drivers/pci/dwc/pcie-designware-ep.c > >> +++ b/drivers/pci/dwc/pcie-designware-ep.c > >> @@ -70,8 +70,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar, > >> u32 free_win; > >> struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > >> > >> - free_win = find_first_zero_bit(&ep->ib_window_map, > >> - sizeof(ep->ib_window_map)); > >> + free_win = ffz(ep->ib_window_map); > > > > You fix the bug at hand but code suffers from the same issue since we > > should be checking the value against ~0UL first ie if size in bits < > > num_ib_windows (that I think it is unlikely anyway but while at it let's > > think if we can improve this). > > > > @Kishon: maybe we should add a static upper size and declare a > > static bitmap accordingly (and still use find_first_zero_bit()) ? > > > > I think the check against ~0UL should be added anyway, let's get > > Kishon's opinion before a v3 (if any). > > Yeah, for both ffz and find_first_zero_bit, the results are undefined if no > zero exists. > > I'm thinking if we should just use > find_first_zero_bit(&ep->ib_window_map, ep->num_ib_windows) after checking if > there are some '0' bits left. Well yes but first the &ep->ib_window_map must be sized accordingly. Lorenzo ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] PCI: endpoint: Fix find_first_zero_bit() usage 2017-11-17 15:00 [PATCH v2 0/2] Fix find_first_zero_bit() usage Niklas Cassel 2017-11-17 15:00 ` [PATCH v2 1/2] PCI: designware-ep: " Niklas Cassel @ 2017-11-17 15:00 ` Niklas Cassel 1 sibling, 0 replies; 6+ messages in thread From: Niklas Cassel @ 2017-11-17 15:00 UTC (permalink / raw) To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Bjorn Helgaas Cc: Niklas Cassel, linux-pci, linux-kernel find_first_zero_bit()'s parameter 'size' is defined in bits, not in bytes. Calling find_first_zero_bit() with the wrong size unit will lead to insidious bugs. Fix this by using replacing find_first_zero_bit() with ffz(), since ffz() only works on a single 'unsigned long' and therefore does not need a size argument. Fixes: d74679911610 ("PCI: endpoint: Introduce configfs entry for configuring EP functions") Signed-off-by: Niklas Cassel <niklas.cassel@axis.com> --- drivers/pci/endpoint/pci-ep-cfs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c index 4f74386c1ced..96b984685640 100644 --- a/drivers/pci/endpoint/pci-ep-cfs.c +++ b/drivers/pci/endpoint/pci-ep-cfs.c @@ -108,8 +108,7 @@ static int pci_epc_epf_link(struct config_item *epc_item, if (ret) goto err_add_epf; - func_no = find_first_zero_bit(&epc_group->function_num_map, - sizeof(epc_group->function_num_map)); + func_no = ffz(epc_group->function_num_map); set_bit(func_no, &epc_group->function_num_map); epf->func_no = func_no; -- 2.14.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-21 16:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-17 15:00 [PATCH v2 0/2] Fix find_first_zero_bit() usage Niklas Cassel 2017-11-17 15:00 ` [PATCH v2 1/2] PCI: designware-ep: " Niklas Cassel 2017-11-17 18:43 ` Lorenzo Pieralisi 2017-11-21 7:54 ` Kishon Vijay Abraham I 2017-11-21 16:44 ` Lorenzo Pieralisi 2017-11-17 15:00 ` [PATCH v2 2/2] PCI: endpoint: " Niklas Cassel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).