linux-kernel - Re: [PATCH] PCI: rcar-host: Avoid objtool no-cfi warning in rcar_pcie

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: <uebexl7d5gfjopb26gstftahu2ouab3ekonw4dzgegw3on5cwr@vqc2zmxiluvt>
Date: Tue, 14 Oct 2025 18:32:59 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>, 
	Marek Vasut <marek.vasut+renesas@...il.com>, Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Geert Uytterhoeven <geert+renesas@...der.be>, Magnus Damm <magnus.damm@...il.com>, linux-pci@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org, llvm@...ts.linux.dev, 
	kernel test robot <lkp@...el.com>, Kees Cook <kees@...nel.org>, 
	"Peter Zijlstra (Intel)" <peterz@...radead.org>
Subject: Re: [PATCH] PCI: rcar-host: Avoid objtool no-cfi warning in
 rcar_pcie_probe()

On Tue, Oct 14, 2025 at 01:32:09AM -0700, Nathan Chancellor wrote:
> Hi Geert,
> 
> On Tue, Oct 14, 2025 at 09:16:58AM +0200, Geert Uytterhoeven wrote:
> > On Mon, 13 Oct 2025 at 20:26, Nathan Chancellor <nathan@...nel.org> wrote:
> > > ---
> > > Another alternative is to make this driver depend on CONFIG_OF since it
> > > clearly requires it but that would restrict compile testing so I went
> > > with this first.
> > > ---
> > >  drivers/pci/controller/pcie-rcar-host.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
> > > index 213028052aa5..15514c9c1927 100644
> > > --- a/drivers/pci/controller/pcie-rcar-host.c
> > > +++ b/drivers/pci/controller/pcie-rcar-host.c
> > > @@ -981,7 +981,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> > >                 goto err_clk_disable;
> > >
> > >         host->phy_init_fn = of_device_get_match_data(dev);
> > > -       err = host->phy_init_fn(host);
> > > +       err = host->phy_init_fn ? host->phy_init_fn(host) : -ENODEV;
> > >         if (err) {
> > >                 dev_err(dev, "failed to init PCIe PHY\n");
> > >                 goto err_clk_disable;
> > 
> > I am afraid you're playing a big game of whack-a-mole, since we tend
> > to remove these checks, as they can never happen in practice (driver
> > is probed from DT only, and all entries in rcar_pcie_of_match[] have
> > a non-NULL .data member)...
> 
> Thanks for the input! Yeah, that is fair, as I alluded to in the scissor
> area. We could just do
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 41748d083b93..d8688abc5b27 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -243,6 +243,7 @@ config PCI_TEGRA
>  config PCIE_RCAR_HOST
>  	bool "Renesas R-Car PCIe controller (host mode)"
>  	depends on ARCH_RENESAS || COMPILE_TEST
> +	depends on OF
>  	depends on PCI_MSI
>  	select IRQ_MSI_LIB
>  	help
> 
> since it is required for the driver to function. Another alternative
> would be something like either:
> 
> diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
> index 213028052aa5..c237e04392e6 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -941,6 +941,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	u32 data;
>  	int err;
>  
> +	if (!IS_ENABLED(CONFIG_OF))
> +		return -ENODEV;
> +
>  	bridge = devm_pci_alloc_host_bridge(dev, sizeof(*host));
>  	if (!bridge)
>  		return -ENOMEM;
> 
> or
> 
> diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
> index 213028052aa5..2aee2e0d9a1d 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -980,8 +980,12 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  	if (err)
>  		goto err_clk_disable;
>  
> -	host->phy_init_fn = of_device_get_match_data(dev);
> -	err = host->phy_init_fn(host);
> +	if (IS_ENABLED(CONFIG_OF)) {
> +		host->phy_init_fn = of_device_get_match_data(dev);
> +		err = host->phy_init_fn(host);
> +	} else {
> +		err = -ENODEV;
> +	}
>  	if (err) {
>  		dev_err(dev, "failed to init PCIe PHY\n");
>  		goto err_clk_disable;
> 
> to keep the ability to compile test the driver without CONFIG_OF while
> having no impact on the final object code and avoiding the NULL call. I
> am open to other thoughts and ideas as well.
> 

TBH, I hate both of these CONFIG_OF checks as most of the controller drivers
are just OF drivers. If we were to sprinkle CONFIG_OF check, then it has to be
done in almost all controller drivers (except VMD, Hyper-V).

If compiler is getting smart enough to detect these NULL invocations, then it
may start to trigger the same issue for other OF APIs as well. So I'd prefer to
have the OF dependency in Kconfig, sacrificing COMPILE_TEST on non-OF configs.

- Mani

-- 
மணிவண்ணன் சதாசிவம்