Re: [PATCH dovetail 6.x] x86: dovetail: Fix inband IRQ state corruption on out-of-band compat syscall

[Philippe Gerum <rpm@xenomai.org>]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> On Fri, 2024-10-11 at 16:36 +0200, Jan Kiszka wrote:
>> On 11.10.24 16:29, Philippe Gerum wrote:
>> > Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>> > 
>> > > On Thu, 2024-10-10 at 18:07 +0200, Philippe Gerum wrote:
>> > > > Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>> > > > 
>> > > > > The following kernel splat was found when running the Xenomai 3
>> > > > > testsuite in compat on dovetail enabled kernels:
>> > > > > 
>> > > > > [  513.620975] IRQ pipeline: some code running in oob context 'Xenomai'
>> > > > >                              called an in-band only routine
>> > > > > [  513.620998] CPU: 0 PID: 510 Comm: smokey Not tainted 6.10.0+ #76
>> > > > > [  513.621003] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
>> > > > > [  513.621005] IRQ stage: Xenomai
>> > > > > [  513.621007] Call Trace:
>> > > > > [  513.621011]  <TASK>
>> > > > > [  513.621015]  dump_stack_lvl+0x6f/0xd0
>> > > > > [  513.621202]  __inband_irq_enable+0xb/0x60
>> > > > > [  513.621249]  do_int80_emulation+0x68/0x160
>> > > > > [  513.621265]  asm_int80_emulation+0x1a/0x20
>> > > > > [  513.621285] RIP: 0023:0xf7f692ba
>> > > > > [  513.621288] Code: Unable to access opcode bytes at 0xf7f69290.
>> > > > > [  513.621303] RSP: 002b:00000000ffeedf40 EFLAGS: 00000282 ORIG_RAX: 0000000000000127
>> > > > > [  513.621307] RAX: ffffffffffffffda RBX: 00000000ffffff9c RCX: 00000000ffeedfa0
>> > > > > [  513.621309] RDX: 0000000000088000 RSI: 0000000000000000 RDI: 00000000f7f7aff4
>> > > > > [  513.621311] RBP: 00000000ffeedf88 R08: 0000000000000000 R09: 0000000000000000
>> > > > > [  513.621313] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
>> > > > > [  513.621315] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>> > > > > [  513.621319]  </TASK>
>> > > > > 
>> > > > > When entering the low level entry code from the out-of-band stage
>> > > > > the in-band IRQ state was changed. That is now avoided by calling
>> > > > > syscall_enter_from_user_enable_irqs() as all other entry points do.
>> > > > > 
>> > > > > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
>> > > > > ---
>> > > > >  arch/x86/entry/common.c | 2 +-
>> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > > > > 
>> > > > > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
>> > > > > index 467e2511df2e..13589f1ca89b 100644
>> > > > > --- a/arch/x86/entry/common.c
>> > > > > +++ b/arch/x86/entry/common.c
>> > > > > @@ -259,7 +259,7 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
>> > > > >  
>> > > > >  	nr = syscall_32_enter(regs);
>> > > > >  
>> > > > > -	local_irq_enable_full();
>> > > > > +	syscall_enter_from_user_enable_irqs();
>> > > > >  	nr = syscall_enter_from_user_mode_work(regs, nr);
>> > > > >  
>> > > > >  	if (dovetailing()) {
>> > > > 
>> > > > Ack. Merged into 6.1.y, 6.6.y and 6.11, dovetail and evl trees.
>> > > 
>> > > Philippe, I have to apologize. The patch is correct for 6.11, but not
>> > > for the older ones. syscall_enter_from_user_enable_irqs() does not
>> > > exist in those branches.
>> > > 
>> > > I have to check why the build error was not properly detected on my
>> > > end. Can't tell yet...
>> > > 
>> > > I will work on backports, meanwhile you should drop it from 6.6 and
>> > > 6.1. Thanks, and sorry.
>> > > 
>> > 
>> > As Jan pointed out, your patch is fine for all 6.x so far, this helper
>> > appears inline upstream in some header only after 6.6.
>> > 
>> 
>> I think the issue is that, depending on some compiler settings, the
>> function may or may not found. It is static inline only for
>> kernel/entry/common.c, thus indeed naturally invisible for
>> arch/x86/entry/common.c.
>> 
>> ../arch/x86/entry/common.c: In function ‘do_int80_emulation’:
>> ../arch/x86/entry/common.c:213:2: error: implicit declaration of
>> function ‘syscall_enter_from_user_enable_irqs’; did you mean
>> ‘syscall_enter_from_user_mode_work’? [-Werror=implicit-function-declaration]
>>   syscall_enter_from_user_enable_irqs();
>>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   syscall_enter_from_user_mode_work
>> cc1: some warnings being treated as errors
>> 
>> Jan
>
> Close... It moved from kernel/entry/common.c to include/linux/entry-
> common.h in newer releases. That's why it's visible.
>
> Seems we have to do the same in a backport...
>

Changing its visibility to extern in kernel/entry/common.c would likely
be enough. There would be no conflict elsewhere.

-- 
Philippe.