drivers/pci/controller/pcie-rcar-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
After commit 894af4a1cde6 ("objtool: Validate kCFI calls"), compile
testing pcie-rcar-host.c with CONFIG_FINEIBT=y and CONFIG_OF=n results
in a no-cfi objtool warning in rcar_pcie_probe():
$ cat allno.config
CONFIG_CFI=y
CONFIG_COMPILE_TEST=y
CONFIG_CPU_MITIGATIONS=y
CONFIG_GENERIC_PHY=y
CONFIG_MITIGATION_RETPOLINE=y
CONFIG_MODULES=y
CONFIG_PCI=y
CONFIG_PCI_MSI=y
CONFIG_PCIE_RCAR_HOST=y
CONFIG_X86_KERNEL_IBT=y
$ make -skj"$(nproc)" ARCH=x86_64 KCONFIG_ALLCONFIG=1 LLVM=1 clean allnoconfig vmlinux
vmlinux.o: warning: objtool: rcar_pcie_probe+0x191: no-cfi indirect call!
When CONFIG_OF is unset, of_device_get_match_data() returns NULL, so
LLVM knows this indirect call has no valid destination and drops the
kCFI setup before the call, triggering the objtool check that makes sure
all indirect calls have kCFI setup.
Check that host->phy_init_fn is not NULL before calling it to avoid the
warning.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510092124.O2IX0Jek-lkp@intel.com/
Reviewed-by: Kees Cook <kees@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
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;
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20251013-rcar_pcie_probe-avoid-nocfi-objtool-warning-1a975accb6b6
Best regards,
--
Nathan Chancellor <nathan@kernel.org>Hi Nathan,
On Mon, 13 Oct 2025 at 20:26, Nathan Chancellor <nathan@kernel.org> wrote:
> After commit 894af4a1cde6 ("objtool: Validate kCFI calls"), compile
> testing pcie-rcar-host.c with CONFIG_FINEIBT=y and CONFIG_OF=n results
> in a no-cfi objtool warning in rcar_pcie_probe():
>
> $ cat allno.config
> CONFIG_CFI=y
> CONFIG_COMPILE_TEST=y
> CONFIG_CPU_MITIGATIONS=y
> CONFIG_GENERIC_PHY=y
> CONFIG_MITIGATION_RETPOLINE=y
> CONFIG_MODULES=y
> CONFIG_PCI=y
> CONFIG_PCI_MSI=y
> CONFIG_PCIE_RCAR_HOST=y
> CONFIG_X86_KERNEL_IBT=y
>
> $ make -skj"$(nproc)" ARCH=x86_64 KCONFIG_ALLCONFIG=1 LLVM=1 clean allnoconfig vmlinux
> vmlinux.o: warning: objtool: rcar_pcie_probe+0x191: no-cfi indirect call!
>
> When CONFIG_OF is unset, of_device_get_match_data() returns NULL, so
> LLVM knows this indirect call has no valid destination and drops the
> kCFI setup before the call, triggering the objtool check that makes sure
> all indirect calls have kCFI setup.
>
> Check that host->phy_init_fn is not NULL before calling it to avoid the
> warning.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202510092124.O2IX0Jek-lkp@intel.com/
> Reviewed-by: Kees Cook <kees@kernel.org>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Thanks for your patch!
> ---
> 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)...
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
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@kernel.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.
Cheers,
Nathan
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@kernel.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
--
மணிவண்ணன் சதாசிவம்
© 2016 - 2025 Red Hat, Inc.