linux-kernel - Re: next-20240712: task_work.c:(.text+0xc2): undefined reference to `irq_work

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: <6494316f-fcf9-4c19-9f3c-f8ad6f0810fa@app.fastmail.com>
Date: Fri, 12 Jul 2024 16:11:54 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Peter Zijlstra" <peterz@...radead.org>
Cc: "Naresh Kamboju" <naresh.kamboju@...aro.org>,
 "open list" <linux-kernel@...r.kernel.org>, lkft-triage@...ts.linaro.org,
 "Linux Regressions" <regressions@...ts.linux.dev>,
 "Dan Carpenter" <dan.carpenter@...aro.org>,
 "Anders Roxell" <anders.roxell@...aro.org>,
 "Sebastian Andrzej Siewior" <bigeasy@...utronix.de>
Subject: Re: next-20240712: task_work.c:(.text+0xc2): undefined reference to
 `irq_work_queue'

On Fri, Jul 12, 2024, at 15:24, Peter Zijlstra wrote:
> On Fri, Jul 12, 2024 at 02:28:38PM +0200, Arnd Bergmann wrote:
>> On Fri, Jul 12, 2024, at 14:13, Naresh Kamboju wrote:
>
>> that we may have to always enable IRQ_WORK even on non-SMP
>> kernels now. In practice it is already enabled in most
>> configurations for one reason or another, the the cost is
>> likely very small.
>> 
>> Otherwise checking for CONFIG_HAVE_NMI in the new code might work.
>
> ARM seems to have HAVE_NMI while also being one of the architectures
> that is now failing.

Right, in this case we would also need

--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -236,6 +236,7 @@ config HAVE_FUNCTION_ERROR_INJECTION
 
 config HAVE_NMI
        bool
+       select IRQ_WORK
 
 config HAVE_FUNCTION_DESCRIPTORS
        bool

> I'm a bit confused though, perf is already depending on irq_work (and
> perf is the user of TWA_NMI_CURRENT). So I'm not exactly sure what
> config fail is leading to all this.

Ok, this is the bit I was missing. If it's only needed for
perf, then the problem is that the caller is built in unconditionally
even when perf is disabled, otherwise it would be caught by
the existing:

config PERF_EVENTS
        bool "Kernel performance events and counters"
        select IRQ_WORK

> I suppose we can do something like the below.. it'll end up in a runtime
> fail for whoever manages to use TWA_NMI_CURRENT without also having
> irq_work enabled, but that should currently be nobody.

>  		break;
> +#ifdef CONFIG_IRQ_WORK
>  	case TWA_NMI_CURRENT:
>  		irq_work_queue(this_cpu_ptr(&irq_work_NMI_resume));
>  		break;
> +#endif
>  	default:

This looks more fragile than necessary. as it might fail if anything
else starts using TWA_NMI_CURRENT without selecting IRQ_WORK.
I would prefer either something that makes it still run into
a compile-time error (e.g. hiding the TWA_NMI_CURRENT inside
the same #ifdef), or just making IRQ_WORK unconditional
as I first suggested.

Configurations without IRQ_WORK are extremely rare, since
it also gets selected by CONFIG_PRINTK, which can only be
turned off for CONFIG_EXPERT=y and is almost always a good
idea even for small kernels.

      Arnd