| Message ID | 20190325213113.24705-1-kjlu@umn.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] pci: pcie-xilinx: fix a missing-check bug for __get_free_pages | expand |
Hi Kangjie, Thanks for the patch! Please update the subject line like this: PCI: xilinx: Check for __get_free_pages() failure You can always get a good idea of the style for subject lines by doing something like this: git log --oneline --follow drivers/pci/controller/pcie-xilinx.c On Mon, Mar 25, 2019 at 04:31:13PM -0500, Kangjie Lu wrote: > In case __get_free_pages fail, the fix returns -ENOMEMto avoid > NULL pointer dereference. s/In case/If/ s/__get_free_pages/__get_free_pages()/ s/fail/fails/ s/the fix returns/return/ s/-ENOMEMto/-ENOMEM to/ > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > Reviewed-by: Steven Price <steven.price@arm.com> I didn't see Steven's reviewed-by on the mailing list. I did see his *review*, but his "Reviewed-by: Steven Price <steven.price@arm.com>" implies that he reviewed it *and* believes it to ready for merging (see Documentation/process/submitting-patches.rst for all the details). So we should only add the Reviewed-by tag after Steven himself posts it. > --- > v2: caller is redefined to accept the error code, as suggested by > Steven Price <steven.price@arm.com> > --- > drivers/pci/controller/pcie-xilinx.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c > index 9bd1a35cd5d8..abc214e94f7c 100644 > --- a/drivers/pci/controller/pcie-xilinx.c > +++ b/drivers/pci/controller/pcie-xilinx.c > @@ -336,14 +336,19 @@ static const struct irq_domain_ops msi_domain_ops = { > * xilinx_pcie_enable_msi - Enable MSI support > * @port: PCIe port information > */ > -static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port) > +static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port) > { > phys_addr_t msg_addr; > > port->msi_pages = __get_free_pages(GFP_KERNEL, 0); > + if (unlikely(!port->msi_pages)) > + return -ENOMEM; No need to use "unlikely()" here. It *is* unlikely that __get_free_pages() will fail, but the annotation clutters the code a bit, so I prefer to avoid it except for performance paths. This should probably be documented somewhere in Documentation/process/, but regrettably, it isn't (yet). > msg_addr = virt_to_phys((void *)port->msi_pages); > pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1); > pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2); > + > + return 0; > } > > /* INTx Functions */ > @@ -498,6 +503,7 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) > struct device *dev = port->dev; > struct device_node *node = dev->of_node; > struct device_node *pcie_intc_node; > + int ret; > > /* Setup INTx */ > pcie_intc_node = of_get_next_child(node, NULL); > @@ -526,7 +532,9 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) > return -ENODEV; > } > > - xilinx_pcie_enable_msi(port); > + ret = xilinx_pcie_enable_msi(port); > + if (ret) > + return ret; > } > > return 0; > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c index 9bd1a35cd5d8..abc214e94f7c 100644 --- a/drivers/pci/controller/pcie-xilinx.c +++ b/drivers/pci/controller/pcie-xilinx.c @@ -336,14 +336,19 @@ static const struct irq_domain_ops msi_domain_ops = { * xilinx_pcie_enable_msi - Enable MSI support * @port: PCIe port information */ -static void xilinx_pcie_enable_msi(struct xilinx_pcie_port *port) +static int xilinx_pcie_enable_msi(struct xilinx_pcie_port *port) { phys_addr_t msg_addr; port->msi_pages = __get_free_pages(GFP_KERNEL, 0); + if (unlikely(!port->msi_pages)) + return -ENOMEM; + msg_addr = virt_to_phys((void *)port->msi_pages); pcie_write(port, 0x0, XILINX_PCIE_REG_MSIBASE1); pcie_write(port, msg_addr, XILINX_PCIE_REG_MSIBASE2); + + return 0; } /* INTx Functions */ @@ -498,6 +503,7 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) struct device *dev = port->dev; struct device_node *node = dev->of_node; struct device_node *pcie_intc_node; + int ret; /* Setup INTx */ pcie_intc_node = of_get_next_child(node, NULL); @@ -526,7 +532,9 @@ static int xilinx_pcie_init_irq_domain(struct xilinx_pcie_port *port) return -ENODEV; } - xilinx_pcie_enable_msi(port); + ret = xilinx_pcie_enable_msi(port); + if (ret) + return ret; } return 0;