linux-kernel - RE: [PATCH] PCI: imx6: install the fault handler only if we are really running on a compatible device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AS8PR04MB8676676FCDB679626CAB81B48CAD9@AS8PR04MB8676.eurprd04.prod.outlook.com>
Date:   Wed, 1 Mar 2023 02:57:47 +0000
From:   Hongxing Zhu <hongxing.zhu@....com>
To:     "H. Nikolaus Schaller" <hns@...delico.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Krzysztof Wilczy��ski <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>
CC:     Rob Herring <robh@...nel.org>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "letux-kernel@...nphoenux.org" <letux-kernel@...nphoenux.org>,
        "kernel@...a-handheld.com" <kernel@...a-handheld.com>
Subject: RE: [PATCH] PCI: imx6: install the fault handler only if we are
 really running on a compatible device


> -----Original Message-----
> From: H. Nikolaus Schaller <hns@...delico.com>
> Sent: 2023年2月28日 16:44
> To: Hongxing Zhu <hongxing.zhu@....com>; Lucas Stach
> <l.stach@...gutronix.de>; Lorenzo Pieralisi <lpieralisi@...nel.org>; Krzysztof
> Wilczy��ski <kw@...ux.com>; Bjorn Helgaas <bhelgaas@...gle.com>; Shawn
> Guo <shawnguo@...nel.org>; Sascha Hauer <s.hauer@...gutronix.de>
> Cc: Rob Herring <robh@...nel.org>; Pengutronix Kernel Team
> <kernel@...gutronix.de>; Fabio Estevam <festevam@...il.com>; dl-linux-imx
> <linux-imx@....com>; linux-pci@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org;
> letux-kernel@...nphoenux.org; kernel@...a-handheld.com; H. Nikolaus
> Schaller <hns@...delico.com>
> Subject: [PATCH] PCI: imx6: install the fault handler only if we are really running
> on a compatible device
> 
> commit bb38919ec56e ("PCI: imx6: Add support for i.MX6 PCIe controller")
> added a fault hook to this driver in the probe function. So it was only installed if
> needed.
> 
> commit bde4a5a00e76 ("PCI: imx6: Allow probe deferral by reset GPIO") moved
> it from probe to driver init which installs the hook unconditionally as soon as the
> driver is compiled into a kernel.
> 
> When this driver is compiled as a module, the hook is not registered until after
> the driver has been matched with a .compatible and loaded.
> 
> commit 415b6185c541 ("PCI: imx6: Fix config read timeout handling") extended
> the fault handling code.
> 
> commit 2d8ed461dbc9 ("PCI: imx6: Add support for i.MX8MQ") added some
> protection for non-ARM architectures, but this does not protect non-i.MX ARM
> architectures.
> 
> Since fault handlers can be triggered on any architecture for different reasons,
> there is no guarantee that they will be triggered only for the assumed situation,
> leading to improper error handling (i.MX6-specific
> imx6q_pcie_abort_handler) on foreign systems.
> 
> I had seen strange L3 imprecise external abort messages several times on
> OMAP4 and OMAP5 devices and couldn't make sense of them until I realized
> they were related to this unused imx6q driver because I had
> CONFIG_PCI_IMX6=y.
> 
> Note that CONFIG_PCI_IMX6=y is useful for kernel binaries that are designed to
> run on different ARM SoC and be differentiated only by device tree binaries. So
> turning off CONFIG_PCI_IMX6 is not a solution.
> 
> Therefore we check the compatible in the init function before registering the
> fault handler.
> 
> Fixes: bde4a5a00e76 ("PCI: imx6: Allow probe deferral by reset GPIO")
> Fixes: 415b6185c541 ("PCI: imx6: Fix config read timeout handling")
> Fixes: 2d8ed461dbc9 ("PCI: imx6: Add support for i.MX8MQ")
> 
> Signed-off-by: H. Nikolaus Schaller <hns@...delico.com>
Hi H.Nikolaus:
I'm fine with these changes. Thanks.
Reviewed-by: Richard Zhu <hongxing.zhu@....com>

Best Regards
Richard Zhu
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> b/drivers/pci/controller/dwc/pci-imx6.c
> index 1dde5c579edc8..89774aa187ae8 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1402,6 +1402,15 @@
> DECLARE_PCI_FIXUP_CLASS_HEADER(PCI_VENDOR_ID_SYNOPSYS, 0xabcd,
> static int __init imx6_pcie_init(void)  {  #ifdef CONFIG_ARM
> +	const struct of_device_id *reboot_id;
> +	struct device_node *np;
> +
> +	np = of_find_matching_node_and_match(NULL, imx6_pcie_of_match,
> +					     &reboot_id);
> +	if (!np)
> +		return -ENODEV;
> +	of_node_put(np);
> +
>  	/*
>  	 * Since probe() can be deferred we need to make sure that
>  	 * hook_fault_code is not called after __init memory is freed
> --
> 2.38.1