linux-kernel - Re: [tip:x86/core 1/1] vmlinux.o: warning: objtool: 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] [day] [month] [year] [list]

Message-ID: <20251013082629.GH4067720@noisy.programming.kicks-ass.net>
Date: Mon, 13 Oct 2025 10:26:29 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Nathan Chancellor <nathan@...nel.org>
Cc: llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	Ingo Molnar <mingo@...nel.org>, Kees Cook <kees@...nel.org>,
	kernel test robot <lkp@...el.com>
Subject: Re: [tip:x86/core 1/1] vmlinux.o: warning: objtool:
 rcar_pcie_probe+0x13e: no-cfi indirect call!

On Fri, Oct 10, 2025 at 03:30:12PM -0700, Nathan Chancellor wrote:
> On Fri, Oct 10, 2025 at 09:44:46AM +0200, Peter Zijlstra wrote:
> > That's here... and that is indeed broken. Also note how it zeros r11
> > right before calling it.
> > 
> > AFAICT this is:
> > 
> >         host->phy_init_fn = of_device_get_match_data(dev);
> >         err = host->phy_init_fn(host);
> > 
> > Where it has decided that of_device_get_match_data() *will* return NULL
> > and then helpfully emits (*NULL)(); or something like that. And then
> 
> Oh duh because it will :)
> 
>   $ rg '^(# )?CONFIG_OF' .config
>   1528:# CONFIG_OF is not set
> 
> which means that of_device_get_match_data() is always NULL:
> 
>   static inline const void *of_device_get_match_data(const struct device *dev)
>   {
>       return NULL;
>   }
> 
> > forgets to add CFI bits on for extra fun and games.
> 
> which means this is another instance of what Sami mentioned happening on
> another report of a similar issue
> 
>   https://lore.kernel.org/CABCJKue1wCB6jBLYUc-fAEzpyQWHXwbk8R5GBaZCkCao0EQZPA@mail.gmail.com/

Ah yes -- I had missed that :/

> which does somewhat make sense because what's the point of setting up
> the CFI call if you know nothing can actually make use of it since we
> will crash when trying to indirectly call a NULL pointer?

As Sami says, it would be really nice if clang would at least WARN about
emitting an unconditional NULL call like that. I mean, it *knows* its
going to crash and burn at that point, right?

> Something like this would avoid this issue then.

Yes, this seems reasonable -- even if the driver should perhaps
mandate/depend on CONFIG_OF, making sure to behave when NULL does get
returned is definitely a good thing!.

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>

> Cheers,
> Nathan
> 
> 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;