Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Wed, 22 Jun 2022 09:45:49 +0200 | Subject | Re: [PATCH 2/2] PCI: rcar: Resolve of_find_matching_node() reference leak |
| |
Hi Bjorn,
On Wed, Jun 22, 2022 at 4:57 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > Previously, rcar_pcie_init() used of_find_matching_node() to search the > entire device tree for compatible strings for which we need to install an > abort handler. If we found one, we got a device_node with refcount > incremented, but we discarded the pointer and never released that > reference. > > Extend the struct rcar_variant to indicate whether each variant requires an > abort handler. Install the handler in rcar_pcie_probe() when needed. > > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Cc: Liang He <windhl@126.com> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Thanks for your patch!
> --- a/drivers/pci/controller/pcie-rcar-host.c > +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -964,12 +965,35 @@ static int rcar_pcie_parse_map_dma_ranges(struct rcar_pcie_host *host) > return err; > } > > +#ifdef CONFIG_ARM > +static int rcar_pcie_aarch32_abort_handler(unsigned long addr, > + unsigned int fsr, struct pt_regs *regs) > +{ > + return !fixup_exception(regs); > +} > +#endif > + > +static void rcar_pcie_hook_aborts(void) > +{ > +#ifdef CONFIG_ARM > +#ifdef CONFIG_ARM_LPAE > + hook_fault_code(17, rcar_pcie_aarch32_abort_handler, SIGBUS, 0, > + "asynchronous external abort"); > +#else > + hook_fault_code(22, rcar_pcie_aarch32_abort_handler, SIGBUS, 0, > + "imprecise external abort"); > +#endif > +#endif > +} > + > static const struct rcar_variant rcar_h1_data = { > .phy_init_fn = rcar_pcie_phy_init_h1, > + .hook_aborts = true, > }; > > static const struct rcar_variant rcar_gen2_data = { > .phy_init_fn = rcar_pcie_phy_init_gen2, > + .hook_aborts = true, > }; > > static const struct rcar_variant rcar_gen3_data = { > @@ -1035,6 +1059,9 @@ static int rcar_pcie_probe(struct platform_device *pdev) > goto err_clk_disable; > } > > + if (host->variant->hook_aborts) > + rcar_pcie_hook_aborts();
I was quite sure there was a good reason why this was not done in .probe() before...
And indeed, the original submission[1] did have a comment explaining that:
+ /* + * Since probe() can be deferred we need to make sure that + * hook_fault_code is not called after __init memory is freed + * by kernel and since rcar_pcie_abort_handler() is a no-op, + * we can install the handler here without risking it + * accessing some uninitialized driver state. + */
No idea why it was removed in v2 and later, but the point is: hook_fault_code() is __init, so you cannot call it from a deferred probe. And you should have got a section mismatch warning ;-)
[1] https://lore.kernel.org/all/20200912211853.15321-1-marek.vasut@gmail.com/
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |