[linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[kernel test robot]

hi, Thomas Gleixner,

as we understand, this commit be5341eb0d doesn't introduce new WARNING, it just
converts the WARING in another type.


b82a8dbd3d2f4563 be5341eb0d43b1e754799498bd2
---------------- ---------------------------
       fail:runs  %reproduction    fail:runs
           |             |             |
          9:9         -100%            :9     dmesg.WARNING:CPU:#PID:#at_do_int80_syscall_32
           :9          100%           9:9     dmesg.WARNING:CPU:#PID:#at_int80_emulation


however, we failed to bisect dmesg.WARNING:CPU:#PID:#at_do_int80_syscall_32.
at the same time, we are not sure enough if this WARNING worth to care? or it's
just due to our test config (we used a randconfig in this test which could be
got from below link).

so we just report to you what we observed in our tests, in case it's still
expose some potential problems, and maybe you could find correct owner to make
some code fix.


Hello,

kernel test robot noticed "WARNING:CPU:#PID:#at_int80_emulation" on:

commit: be5341eb0d43b1e754799498bd2e8756cc167a41 ("x86/entry: Convert INT 0x80 emulation to IDTENTRY")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

[test failed on linux-next/master 11651f8cb2e88372d4ed523d909514dc9a613ea3]

in testcase: boot

compiler: clang-16
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)



[ 13.481107][ T48] WARNING: CPU: 0 PID: 48 at int80_emulation (arch/x86/entry/common.c:164) 
[   13.481454][   T48] Modules linked in:
[   13.481655][   T48] CPU: 0 PID: 48 Comm: init Tainted: G                 N 6.7.0-rc4-00002-gbe5341eb0d43 #1
[ 13.482162][ T48] RIP: 0010:int80_emulation (arch/x86/entry/common.c:164) 
[ 13.482437][ T48] Code: 01 00 00 77 43 89 c1 48 81 f9 c9 01 00 00 48 19 c9 21 c1 48 89 df 4c 8b 1c cd 90 12 20 9a 41 ba 27 cb d4 4f 45 03 53 fc 74 02 <0f> 0b 41 ff d3 48 89 c1 48 89 4b 50 90 48 89 df 5b 41 5e 31 c0 31
All code
========
   0:	01 00                	add    %eax,(%rax)
   2:	00 77 43             	add    %dh,0x43(%rdi)
   5:	89 c1                	mov    %eax,%ecx
   7:	48 81 f9 c9 01 00 00 	cmp    $0x1c9,%rcx
   e:	48 19 c9             	sbb    %rcx,%rcx
  11:	21 c1                	and    %eax,%ecx
  13:	48 89 df             	mov    %rbx,%rdi
  16:	4c 8b 1c cd 90 12 20 	mov    -0x65dfed70(,%rcx,8),%r11
  1d:	9a 
  1e:	41 ba 27 cb d4 4f    	mov    $0x4fd4cb27,%r10d
  24:	45 03 53 fc          	add    -0x4(%r11),%r10d
  28:	74 02                	je     0x2c
  2a:*	0f 0b                	ud2		<-- trapping instruction
  2c:	41 ff d3             	call   *%r11
  2f:	48 89 c1             	mov    %rax,%rcx
  32:	48 89 4b 50          	mov    %rcx,0x50(%rbx)
  36:	90                   	nop
  37:	48 89 df             	mov    %rbx,%rdi
  3a:	5b                   	pop    %rbx
  3b:	41 5e                	pop    %r14
  3d:	31 c0                	xor    %eax,%eax
  3f:	31                   	.byte 0x31

Code starting with the faulting instruction
===========================================
   0:	0f 0b                	ud2
   2:	41 ff d3             	call   *%r11
   5:	48 89 c1             	mov    %rax,%rcx
   8:	48 89 4b 50          	mov    %rcx,0x50(%rbx)
   c:	90                   	nop
   d:	48 89 df             	mov    %rbx,%rdi
  10:	5b                   	pop    %rbx
  11:	41 5e                	pop    %r14
  13:	31 c0                	xor    %eax,%eax
  15:	31                   	.byte 0x31
[   13.483405][   T48] RSP: 0000:ffff9d1ffbbd7f40 EFLAGS: 00010a92
[   13.483713][   T48] RAX: 000000000000001b RBX: ffff9d1ffbbd7f58 RCX: 000000000000001b
[   13.484112][   T48] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9d1ffbbd7f58
[   13.484510][   T48] RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000002b
[   13.484910][   T48] R10: 00000000c26707b6 R11: ffffffff993719d0 R12: 0000000000000000
[   13.485314][   T48] R13: 0000000000000000 R14: 000000000000001b R15: 0000000000000000
[   13.485715][   T48] FS:  0000000000000000(0000) GS:ffffffff9a85c000(0063) knlGS:00000000f7fc7040
[   13.486168][   T48] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
[   13.486500][   T48] CR2: 00000000f7fc2204 CR3: 000000013b76a000 CR4: 00000000000406f0
[   13.486901][   T48] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   13.487299][   T48] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   13.487697][   T48] Call Trace:
[   13.487867][   T48]  <TASK>
[ 13.488018][ T48] ? __warn (kernel/panic.c:236 kernel/panic.c:677) 
[ 13.488229][ T48] ? int80_emulation (arch/x86/entry/common.c:164) 
[ 13.488474][ T48] ? report_cfi_failure (kernel/cfi.c:22) 
[ 13.488731][ T48] ? handle_cfi_failure (arch/x86/kernel/cfi.c:80) 
[ 13.488997][ T48] ? __cfi_sys_ni_posix_timers (kernel/time/posix-stubs.c:27) 
[ 13.489294][ T48] ? handle_bug (arch/x86/kernel/traps.c:238) 
[ 13.489514][ T48] ? exc_invalid_op (arch/x86/kernel/traps.c:258) 
[ 13.489753][ T48] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:568) 
[ 13.490017][ T48] ? __cfi_sys_ni_posix_timers (kernel/time/posix-stubs.c:27) 
[ 13.490306][ T48] ? int80_emulation (arch/x86/entry/common.c:164) 
[ 13.490552][ T48] asm_int80_emulation (arch/x86/include/asm/idtentry.h:573) 
[   13.490804][   T48] RIP: 0023:0xf7ed84cd
[ 13.491012][ T48] Code: eb bb 8d 76 00 83 c4 1c 5b 5e 5f 5d e9 0c f3 f6 ff e8 07 f3 f6 ff eb d4 66 90 66 90 90 89 da 8b 5c 24 04 b8 1b 00 00 00 cd 80 <89> d3 3d 01 f0 ff ff 0f 83 e6 f2 f6 ff c3 66 90 66 90 90 55 e8 4a
All code
========
   0:	eb bb                	jmp    0xffffffffffffffbd
   2:	8d 76 00             	lea    0x0(%rsi),%esi
   5:	83 c4 1c             	add    $0x1c,%esp
   8:	5b                   	pop    %rbx
   9:	5e                   	pop    %rsi
   a:	5f                   	pop    %rdi
   b:	5d                   	pop    %rbp
   c:	e9 0c f3 f6 ff       	jmp    0xfffffffffff6f31d
  11:	e8 07 f3 f6 ff       	call   0xfffffffffff6f31d
  16:	eb d4                	jmp    0xffffffffffffffec
  18:	66 90                	xchg   %ax,%ax
  1a:	66 90                	xchg   %ax,%ax
  1c:	90                   	nop
  1d:	89 da                	mov    %ebx,%edx
  1f:	8b 5c 24 04          	mov    0x4(%rsp),%ebx
  23:	b8 1b 00 00 00       	mov    $0x1b,%eax
  28:	cd 80                	int    $0x80
  2a:*	89 d3                	mov    %edx,%ebx		<-- trapping instruction
  2c:	3d 01 f0 ff ff       	cmp    $0xfffff001,%eax
  31:	0f 83 e6 f2 f6 ff    	jae    0xfffffffffff6f31d
  37:	c3                   	ret
  38:	66 90                	xchg   %ax,%ax
  3a:	66 90                	xchg   %ax,%ax
  3c:	90                   	nop
  3d:	55                   	push   %rbp
  3e:	e8                   	.byte 0xe8
  3f:	4a                   	rex.WX

Code starting with the faulting instruction
===========================================
   0:	89 d3                	mov    %edx,%ebx
   2:	3d 01 f0 ff ff       	cmp    $0xfffff001,%eax
   7:	0f 83 e6 f2 f6 ff    	jae    0xfffffffffff6f2f3
   d:	c3                   	ret
   e:	66 90                	xchg   %ax,%ax
  10:	66 90                	xchg   %ax,%ax
  12:	90                   	nop
  13:	55                   	push   %rbp
  14:	e8                   	.byte 0xe8
  15:	4a                   	rex.WX
[   13.491980][   T48] RSP: 002b:00000000fff098ac EFLAGS: 00000286 ORIG_RAX: 000000000000001b
[   13.492402][   T48] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000fff09ad4
[   13.492800][   T48] RDX: 00000000f7fc1e64 RSI: 00000000fff09b60 RDI: 00000000fff09d4c
[   13.493204][   T48] RBP: 00000000fff09ad4 R08: 0000000000000000 R09: 0000000000000000
[   13.493607][   T48] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[   13.494011][   T48] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[   13.494418][   T48]  </TASK>
[   13.494576][   T48] irq event stamp: 1181
[ 13.494786][ T48] hardirqs last enabled at (1191): console_unlock (arch/x86/include/asm/irqflags.h:19 arch/x86/include/asm/irqflags.h:67 arch/x86/include/asm/irqflags.h:127 kernel/printk/printk.c:341 kernel/printk/printk.c:2706 kernel/printk/printk.c:3038) 
[ 13.495252][ T48] hardirqs last disabled at (1200): console_unlock (kernel/printk/printk.c:339) 
[ 13.495715][ T48] softirqs last enabled at (808): __irq_exit_rcu (include/linux/sched.h:2463 kernel/softirq.c:615 kernel/softirq.c:634) 
[ 13.496172][ T48] softirqs last disabled at (797): __irq_exit_rcu (include/linux/sched.h:2463 kernel/softirq.c:615 kernel/softirq.c:634) 
[   13.496626][   T48] ---[ end trace 0000000000000000 ]---
[   13.496901][   T48] process 48 (init) attempted a POSIX timer syscall while CONFIG_POSIX_TIMERS is not set
[ 13.517868][ T48] CFI failure at int80_emulation+0x67/0xb0 (target: sys_ni_posix_timers+0x0/0x70; expected type: 0xb02b34d9 


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231219/202312191507.348721d2-oliver.sang@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Borislav Petkov]

On Tue, Dec 19, 2023 at 04:49:14PM +0800, kernel test robot wrote:
> [ 13.481107][ T48] WARNING: CPU: 0 PID: 48 at int80_emulation (arch/x86/entry/common.c:164) 
> [   13.481454][   T48] Modules linked in:
> [   13.481655][   T48] CPU: 0 PID: 48 Comm: init Tainted: G                 N 6.7.0-rc4-00002-gbe5341eb0d43 #1
> [ 13.482162][ T48] RIP: 0010:int80_emulation (arch/x86/entry/common.c:164) 

Looking at the dmesg, I think you missed the most important part - the
preceding line:

[   13.480504][   T48] CFI failure at int80_emulation+0x67/0xb0 (target: sys_ni_posix_timers+0x0/0x70; expected type: 0xb02b34d9)
			^^^^^^^^^^^

[   13.481107][   T48] WARNING: CPU: 0 PID: 48 at int80_emulation+0x67/0xb0
[   13.481454][   T48] Modules linked in:
[   13.481655][   T48] CPU: 0 PID: 48 Comm: init Tainted: G                 N 6.7.0-rc4-00002-gbe5341eb0d43 #1

The CFI bla is also in the stack trace.

Now, decode_cfi_insn() has a comment there which says what the compiler
generates about indirect call checks:

         *   movl    -<id>, %r10d       ; 6 bytes
         *   addl    -4(%reg), %r10d    ; 4 bytes
         *   je      .Ltmp1             ; 2 bytes
         *   ud2                        ; <- regs->ip
         *   .Ltmp1:


and the opcodes you decoded...

> [ 13.482437][ T48] Code: 01 00 00 77 43 89 c1 48 81 f9 c9 01 00 00 48 19 c9 21 c1 48 89 df 4c 8b 1c cd 90 12 20 9a 41 ba 27 cb d4 4f 45 03 53 fc 74 02 <0f> 0b 41 ff d3 48 89 c1 48 89 4b 50 90 48 89 df 5b 41 5e 31 c0 31
> All code
> ========
>    0:	01 00                	add    %eax,(%rax)
>    2:	00 77 43             	add    %dh,0x43(%rdi)
>    5:	89 c1                	mov    %eax,%ecx
>    7:	48 81 f9 c9 01 00 00 	cmp    $0x1c9,%rcx
>    e:	48 19 c9             	sbb    %rcx,%rcx
>   11:	21 c1                	and    %eax,%ecx
>   13:	48 89 df             	mov    %rbx,%rdi
>   16:	4c 8b 1c cd 90 12 20 	mov    -0x65dfed70(,%rcx,8),%r11
>   1d:	9a 
>   1e:	41 ba 27 cb d4 4f    	mov    $0x4fd4cb27,%r10d
>   24:	45 03 53 fc          	add    -0x4(%r11),%r10d
>   28:	74 02                	je     0x2c
>   2a:*	0f 0b                	ud2		<-- trapping instruction

... these guys here, look exactly like what the compiler did issue.

This is the first time I'm looking at this CFI bla but it sounds like it
is trying to compare the syscall target's address of
sys_ni_posix_timers with something it is expecting to call and the
comparison doesn't work out (%r10 is not 0).

There's that special symbol __cfi_sys_ni_posix_timers which also gets
generated...

Someone would need to dig into that whole CFI gunk to figure out why
this is not happy.

Oh well.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Linus Torvalds]

On Tue, 19 Dec 2023 at 01:58, Borislav Petkov <bp@alien8.de> wrote:
>
> Looking at the dmesg, I think you missed the most important part - the
> preceding line:
>
> [   13.480504][   T48] CFI failure at int80_emulation+0x67/0xb0 (target: sys_ni_posix_timers+0x0/0x70; expected type: 0xb02b34d9)
>                         ^^^^^^^^^^^

So I think the issue here is that sys_ni_posix_timers is just linker
alias that is used for any non-implemented posix timer system call.

See:

  #define __SYS_NI(abi, name)                                             \
        SYSCALL_ALIAS(__##abi##_##name, sys_ni_posix_timers);

and this all worked fine when the actual call to this was done in
assembly code that happily just called that function directly and
didn't care about any argument types.

But commit be5341eb0d43 ("x86/entry: Convert INT 0x80 emulation to
IDTENTRY") moved that call from assembly into C, and in the process
ended up enabling CFI for it all, and now the compiler will check that
the function types match. Which they don't, because we use that dummy
function (I don't think they do in general).

I don't know what the best fix is. Either CFI should be turned off for
that call, or we should make sure to generate those NI system calls
with the proper types.

The asm didn't care - as long as the function put -ENOSYS in %rax, it
did the right thing - but the kCFI stuff means that the C code now
cares (and checks) that prototypes etc really match.

Maybe we should just get rid of SYS_NI() _entirely_.

I think the only user is the posix-timers stuff, and everything else
uses COND_SYSCALL(), which actually *generates* all the proper weak
functions with all the proper function signatures, instead of playing
around with linker aliases that don't have them.

Afaik, the only reason the posix timers do that odd alias is because
they want to have that

        pr_err_once("process %d (%s) attempted a POSIX timer syscall "
                    "while CONFIG_POSIX_TIMERS is not set\n",
                    current->pid, current->comm);

which I don't think is really worth it. It goes back to 2016 when the
posix timers subsystem became configurable, and I doubt it is worth it
any more (and it was probably of dubious use even at the time).

But I've not had anything to do with the low-level kCFI stuff, and
I'll leave it to Thomas whether that SYS_NI() mess should just be
removed.

I do like the notion of just removing SYS_NI entirely, replacing it
with the standard COND_SYSCALL() thing (and same for the COMPAT
variables, of course).

Thomas?

               Linus

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Andrew Cooper]

On 19/12/2023 6:20 pm, Linus Torvalds wrote:
> On Tue, 19 Dec 2023 at 01:58, Borislav Petkov <bp@alien8.de> wrote:
>> Looking at the dmesg, I think you missed the most important part - the
>> preceding line:
>>
>> [   13.480504][   T48] CFI failure at int80_emulation+0x67/0xb0 (target: sys_ni_posix_timers+0x0/0x70; expected type: 0xb02b34d9)
>>                         ^^^^^^^^^^^
> So I think the issue here is that sys_ni_posix_timers is just linker
> alias that is used for any non-implemented posix timer system call.

My reading of the original report is that there used to be:

    dmesg.WARNING:CPU:#PID:#at_do_int80_syscall_32

and now there's:

    dmesg.WARNING:CPU:#PID:#at_int80_emulation


i.e. kCFI was broken before, and all we did with be5341eb0d43 was change
the inlining, and therefore how the error was rendered.


> See:
>
>   #define __SYS_NI(abi, name)                                             \
>         SYSCALL_ALIAS(__##abi##_##name, sys_ni_posix_timers);

AFAICT, this is the problem, but it was preexiting too.

This is stuffing a function of type void into an array of function
pointers wanting pt_regs * which is indeed a kCFI violation.

Isn't the fix simply this?

~Andrew

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index fd9d12de7e92..12195164d5a4 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1171,7 +1171,7 @@ asmlinkage long sys_ni_syscall(void);
 
 #endif /* CONFIG_ARCH_HAS_SYSCALL_WRAPPER */
 
-asmlinkage long sys_ni_posix_timers(void);
+asmlinkage long sys_ni_posix_timers(const struct pt_regs *regs);
 
 /*
  * Kernel code should not call syscalls (i.e., sys_xyzyyz()) directly.
diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
index 828aeecbd1e8..29692354a908 100644
--- a/kernel/time/posix-stubs.c
+++ b/kernel/time/posix-stubs.c
@@ -22,7 +22,7 @@
 #include <asm/syscall_wrapper.h>
 #endif
 
-asmlinkage long sys_ni_posix_timers(void)
+asmlinkage long sys_ni_posix_timers(const struct pt_regs *regs)
 {
        pr_err_once("process %d (%s) attempted a POSIX timer syscall "
                    "while CONFIG_POSIX_TIMERS is not set\n",

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Sami Tolvanen]

On Tue, Dec 19, 2023 at 10:21 AM Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> On Tue, 19 Dec 2023 at 01:58, Borislav Petkov <bp@alien8.de> wrote:
> >
> > Looking at the dmesg, I think you missed the most important part - the
> > preceding line:
> >
> > [   13.480504][   T48] CFI failure at int80_emulation+0x67/0xb0 (target: sys_ni_posix_timers+0x0/0x70; expected type: 0xb02b34d9)
> >                         ^^^^^^^^^^^
>
> So I think the issue here is that sys_ni_posix_timers is just linker
> alias that is used for any non-implemented posix timer system call.
>
> See:
>
>   #define __SYS_NI(abi, name)                                             \
>         SYSCALL_ALIAS(__##abi##_##name, sys_ni_posix_timers);
>
> and this all worked fine when the actual call to this was done in
> assembly code that happily just called that function directly and
> didn't care about any argument types.

Yes, that's exactly the issue.

> But commit be5341eb0d43 ("x86/entry: Convert INT 0x80 emulation to
> IDTENTRY") moved that call from assembly into C, and in the process
> ended up enabling CFI for it all, and now the compiler will check that
> the function types match. Which they don't, because we use that dummy
> function (I don't think they do in general).
>
> I don't know what the best fix is. Either CFI should be turned off for
> that call, or we should make sure to generate those NI system calls
> with the proper types.

Probably the easiest fix would be to use SYSCALL_DEFINE0 for
sys_ni_posix_timers, and for architectures that implement syscall
wrappers, change sys_ni_posix_timers references to
__<abi>_sys_ni_posix_timers.

Something like this should fix the issue for x86, but it looks like
arm64, riscv, and s390 would need similar syscall wrapper changes:

diff --git a/arch/x86/include/asm/syscall_wrapper.h
b/arch/x86/include/asm/syscall_wrapper.h
index fd2669b1cb2d..ed38265cad27 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -87,7 +87,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
        }

 #define __SYS_NI(abi, name)                                            \
-       SYSCALL_ALIAS(__##abi##_##name, sys_ni_posix_timers);
+       SYSCALL_ALIAS(__##abi##_##name, __##abi##_sys_ni_posix_timers);

 #ifdef CONFIG_X86_64
 #define __X64_SYS_STUB0(name)                                          \
diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
index 828aeecbd1e8..d58f976ec926 100644
--- a/kernel/time/posix-stubs.c
+++ b/kernel/time/posix-stubs.c
@@ -22,7 +22,7 @@
 #include <asm/syscall_wrapper.h>
 #endif

-asmlinkage long sys_ni_posix_timers(void)
+SYSCALL_DEFINE0(ni_posix_timers)
 {
        pr_err_once("process %d (%s) attempted a POSIX timer syscall "
                    "while CONFIG_POSIX_TIMERS is not set\n",

Sami

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Linus Torvalds]

On Tue, 19 Dec 2023 at 11:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> -asmlinkage long sys_ni_posix_timers(void);
> +asmlinkage long sys_ni_posix_timers(const struct pt_regs *regs);

I don't think it should be asmlinkage. That means "use legacy asm
calling conventions", and for x86-32 that means pass on stack. Which I
don't think these actually are.

I think it's an old artefect, and it doesn't matter for something that
doesn't take any arguments, but when you add an argument it's actively
wrong.

Of course, that argument isn't _used_, so it still doesn't matter, but
if the point is to use the right prototype, I think we should just
make it be

    long sys_ni_posix_timers(const struct pt_regs *regs);

although I think Sami's suggestion is probably nicer.

That said, I still think that just getting rid of this horrid special
case for posix timers is the right thing, and we should just remove
that SYS_NI() alias thing entirely.

          Linus

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Linus Torvalds]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

On Tue, 19 Dec 2023 at 12:17, Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> That said, I still think that just getting rid of this horrid special
> case for posix timers is the right thing, and we should just remove
> that SYS_NI() alias thing entirely.

IOW, something like the attached patch.

It's not extensively tested, but hey, the diffstat looks nice:

  arch/arm64/include/asm/syscall_wrapper.h |  4 ---
  arch/riscv/include/asm/syscall_wrapper.h |  5 ----
  arch/s390/include/asm/syscall_wrapper.h  | 13 +--------
  arch/x86/include/asm/syscall_wrapper.h   | 34 +++---------------------
  kernel/sys_ni.c                          | 14 ++++++++++
  kernel/time/posix-stubs.c                | 45 --------------------------------
  6 files changed, 19 insertions(+), 96 deletions(-)

and it builds in at least a *couple* of configurations, including with
CONFIG_POSIX_TIMERS disabled.

I did *not* check whether it might fix the warning, since I doubt my
user space would even boot without that posix timer support (actually,
honestly, because I'm just lazy and "it _looks_ fine to me" was the
main real thing).

But that SYS_NI() thing really does deserve to die, as it was purely
used as a hack for some random timer system calls.

Comments?

            Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 10456 bytes --]

 arch/arm64/include/asm/syscall_wrapper.h |  4 ---
 arch/riscv/include/asm/syscall_wrapper.h |  5 ----
 arch/s390/include/asm/syscall_wrapper.h  | 13 +--------
 arch/x86/include/asm/syscall_wrapper.h   | 34 +++---------------------
 kernel/sys_ni.c                          | 14 ++++++++++
 kernel/time/posix-stubs.c                | 45 --------------------------------
 6 files changed, 19 insertions(+), 96 deletions(-)

diff --git a/arch/arm64/include/asm/syscall_wrapper.h b/arch/arm64/include/asm/syscall_wrapper.h
index d977713ec0ba..abb57bc54305 100644
--- a/arch/arm64/include/asm/syscall_wrapper.h
+++ b/arch/arm64/include/asm/syscall_wrapper.h
@@ -44,9 +44,6 @@
 		return sys_ni_syscall();						\
 	}
 
-#define COMPAT_SYS_NI(name) \
-	SYSCALL_ALIAS(__arm64_compat_sys_##name, sys_ni_posix_timers);
-
 #endif /* CONFIG_COMPAT */
 
 #define __SYSCALL_DEFINEx(x, name, ...)						\
@@ -81,6 +78,5 @@
 	}
 
 asmlinkage long __arm64_sys_ni_syscall(const struct pt_regs *__unused);
-#define SYS_NI(name) SYSCALL_ALIAS(__arm64_sys_##name, sys_ni_posix_timers);
 
 #endif /* __ASM_SYSCALL_WRAPPER_H */
diff --git a/arch/riscv/include/asm/syscall_wrapper.h b/arch/riscv/include/asm/syscall_wrapper.h
index 1d7942c8a6cb..eeec04b7dae6 100644
--- a/arch/riscv/include/asm/syscall_wrapper.h
+++ b/arch/riscv/include/asm/syscall_wrapper.h
@@ -46,9 +46,6 @@ asmlinkage long __riscv_sys_ni_syscall(const struct pt_regs *);
 		return sys_ni_syscall();						\
 	}
 
-#define COMPAT_SYS_NI(name) \
-	SYSCALL_ALIAS(__riscv_compat_sys_##name, sys_ni_posix_timers);
-
 #endif /* CONFIG_COMPAT */
 
 #define __SYSCALL_DEFINEx(x, name, ...)						\
@@ -82,6 +79,4 @@ asmlinkage long __riscv_sys_ni_syscall(const struct pt_regs *);
 		return sys_ni_syscall();					\
 	}
 
-#define SYS_NI(name) SYSCALL_ALIAS(__riscv_sys_##name, sys_ni_posix_timers);
-
 #endif /* __ASM_SYSCALL_WRAPPER_H */
diff --git a/arch/s390/include/asm/syscall_wrapper.h b/arch/s390/include/asm/syscall_wrapper.h
index 9286430fe729..35c1d1b860d8 100644
--- a/arch/s390/include/asm/syscall_wrapper.h
+++ b/arch/s390/include/asm/syscall_wrapper.h
@@ -63,10 +63,6 @@
 	cond_syscall(__s390x_sys_##name);				\
 	cond_syscall(__s390_sys_##name)
 
-#define SYS_NI(name)							\
-	SYSCALL_ALIAS(__s390x_sys_##name, sys_ni_posix_timers);		\
-	SYSCALL_ALIAS(__s390_sys_##name, sys_ni_posix_timers)
-
 #define COMPAT_SYSCALL_DEFINEx(x, name, ...)						\
 	long __s390_compat_sys##name(struct pt_regs *regs);				\
 	ALLOW_ERROR_INJECTION(__s390_compat_sys##name, ERRNO);				\
@@ -85,15 +81,11 @@
 
 /*
  * As some compat syscalls may not be implemented, we need to expand
- * COND_SYSCALL_COMPAT in kernel/sys_ni.c and COMPAT_SYS_NI in
- * kernel/time/posix-stubs.c to cover this case as well.
+ * COND_SYSCALL_COMPAT in kernel/sys_ni.c to cover this case as well.
  */
 #define COND_SYSCALL_COMPAT(name)					\
 	cond_syscall(__s390_compat_sys_##name)
 
-#define COMPAT_SYS_NI(name)						\
-	SYSCALL_ALIAS(__s390_compat_sys_##name, sys_ni_posix_timers)
-
 #define __S390_SYS_STUBx(x, name, ...)						\
 	long __s390_sys##name(struct pt_regs *regs);				\
 	ALLOW_ERROR_INJECTION(__s390_sys##name, ERRNO);				\
@@ -124,9 +116,6 @@
 #define COND_SYSCALL(name)						\
 	cond_syscall(__s390x_sys_##name)
 
-#define SYS_NI(name)							\
-	SYSCALL_ALIAS(__s390x_sys_##name, sys_ni_posix_timers)
-
 #define __S390_SYS_STUBx(x, fullname, name, ...)
 
 #endif /* CONFIG_COMPAT */
diff --git a/arch/x86/include/asm/syscall_wrapper.h b/arch/x86/include/asm/syscall_wrapper.h
index fd2669b1cb2d..21f9407be5d3 100644
--- a/arch/x86/include/asm/syscall_wrapper.h
+++ b/arch/x86/include/asm/syscall_wrapper.h
@@ -86,9 +86,6 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 		return sys_ni_syscall();				\
 	}
 
-#define __SYS_NI(abi, name)						\
-	SYSCALL_ALIAS(__##abi##_##name, sys_ni_posix_timers);
-
 #ifdef CONFIG_X86_64
 #define __X64_SYS_STUB0(name)						\
 	__SYS_STUB0(x64, sys_##name)
@@ -100,13 +97,10 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 #define __X64_COND_SYSCALL(name)					\
 	__COND_SYSCALL(x64, sys_##name)
 
-#define __X64_SYS_NI(name)						\
-	__SYS_NI(x64, sys_##name)
 #else /* CONFIG_X86_64 */
 #define __X64_SYS_STUB0(name)
 #define __X64_SYS_STUBx(x, name, ...)
 #define __X64_COND_SYSCALL(name)
-#define __X64_SYS_NI(name)
 #endif /* CONFIG_X86_64 */
 
 #if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
@@ -120,13 +114,10 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 #define __IA32_COND_SYSCALL(name)					\
 	__COND_SYSCALL(ia32, sys_##name)
 
-#define __IA32_SYS_NI(name)						\
-	__SYS_NI(ia32, sys_##name)
 #else /* CONFIG_X86_32 || CONFIG_IA32_EMULATION */
 #define __IA32_SYS_STUB0(name)
 #define __IA32_SYS_STUBx(x, name, ...)
 #define __IA32_COND_SYSCALL(name)
-#define __IA32_SYS_NI(name)
 #endif /* CONFIG_X86_32 || CONFIG_IA32_EMULATION */
 
 #ifdef CONFIG_IA32_EMULATION
@@ -135,8 +126,7 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
  * additional wrappers (aptly named __ia32_sys_xyzzy) which decode the
  * ia32 regs in the proper order for shared or "common" syscalls. As some
  * syscalls may not be implemented, we need to expand COND_SYSCALL in
- * kernel/sys_ni.c and SYS_NI in kernel/time/posix-stubs.c to cover this
- * case as well.
+ * kernel/sys_ni.c to cover this case as well.
  */
 #define __IA32_COMPAT_SYS_STUB0(name)					\
 	__SYS_STUB0(ia32, compat_sys_##name)
@@ -148,14 +138,10 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 #define __IA32_COMPAT_COND_SYSCALL(name)				\
 	__COND_SYSCALL(ia32, compat_sys_##name)
 
-#define __IA32_COMPAT_SYS_NI(name)					\
-	__SYS_NI(ia32, compat_sys_##name)
-
 #else /* CONFIG_IA32_EMULATION */
 #define __IA32_COMPAT_SYS_STUB0(name)
 #define __IA32_COMPAT_SYS_STUBx(x, name, ...)
 #define __IA32_COMPAT_COND_SYSCALL(name)
-#define __IA32_COMPAT_SYS_NI(name)
 #endif /* CONFIG_IA32_EMULATION */
 
 
@@ -175,13 +161,10 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 #define __X32_COMPAT_COND_SYSCALL(name)					\
 	__COND_SYSCALL(x64, compat_sys_##name)
 
-#define __X32_COMPAT_SYS_NI(name)					\
-	__SYS_NI(x64, compat_sys_##name)
 #else /* CONFIG_X86_X32_ABI */
 #define __X32_COMPAT_SYS_STUB0(name)
 #define __X32_COMPAT_SYS_STUBx(x, name, ...)
 #define __X32_COMPAT_COND_SYSCALL(name)
-#define __X32_COMPAT_SYS_NI(name)
 #endif /* CONFIG_X86_X32_ABI */
 
 
@@ -212,17 +195,12 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 
 /*
  * As some compat syscalls may not be implemented, we need to expand
- * COND_SYSCALL_COMPAT in kernel/sys_ni.c and COMPAT_SYS_NI in
- * kernel/time/posix-stubs.c to cover this case as well.
+ * COND_SYSCALL_COMPAT in kernel/sys_ni.c to cover this case as well.
  */
 #define COND_SYSCALL_COMPAT(name) 					\
 	__IA32_COMPAT_COND_SYSCALL(name)				\
 	__X32_COMPAT_COND_SYSCALL(name)
 
-#define COMPAT_SYS_NI(name)						\
-	__IA32_COMPAT_SYS_NI(name)					\
-	__X32_COMPAT_SYS_NI(name)
-
 #endif /* CONFIG_COMPAT */
 
 #define __SYSCALL_DEFINEx(x, name, ...)					\
@@ -243,8 +221,8 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
  * As the generic SYSCALL_DEFINE0() macro does not decode any parameters for
  * obvious reasons, and passing struct pt_regs *regs to it in %rdi does not
  * hurt, we only need to re-define it here to keep the naming congruent to
- * SYSCALL_DEFINEx() -- which is essential for the COND_SYSCALL() and SYS_NI()
- * macros to work correctly.
+ * SYSCALL_DEFINEx() -- which is essential for the COND_SYSCALL() macro
+ * to work correctly.
  */
 #define SYSCALL_DEFINE0(sname)						\
 	SYSCALL_METADATA(_##sname, 0);					\
@@ -257,10 +235,6 @@ extern long __ia32_sys_ni_syscall(const struct pt_regs *regs);
 	__X64_COND_SYSCALL(name)					\
 	__IA32_COND_SYSCALL(name)
 
-#define SYS_NI(name)							\
-	__X64_SYS_NI(name)						\
-	__IA32_SYS_NI(name)
-
 
 /*
  * For VSYSCALLS, we need to declare these three syscalls with the new
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index e1a6e3c675c0..9a846439b36a 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -201,6 +201,20 @@ COND_SYSCALL(recvmmsg_time32);
 COND_SYSCALL_COMPAT(recvmmsg_time32);
 COND_SYSCALL_COMPAT(recvmmsg_time64);
 
+/* Posix timer syscalls may be configured out */
+COND_SYSCALL(timer_create);
+COND_SYSCALL(timer_gettime);
+COND_SYSCALL(timer_getoverrun);
+COND_SYSCALL(timer_settime);
+COND_SYSCALL(timer_delete);
+COND_SYSCALL(clock_adjtime);
+COND_SYSCALL(getitimer);
+COND_SYSCALL(setitimer);
+COND_SYSCALL(alarm);
+COND_SYSCALL_COMPAT(timer_create);
+COND_SYSCALL_COMPAT(getitimer);
+COND_SYSCALL_COMPAT(setitimer);
+
 /*
  * Architecture specific syscalls: see further below
  */
diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
index 828aeecbd1e8..9b6fcb8d85e7 100644
--- a/kernel/time/posix-stubs.c
+++ b/kernel/time/posix-stubs.c
@@ -17,40 +17,6 @@
 #include <linux/time_namespace.h>
 #include <linux/compat.h>
 
-#ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER
-/* Architectures may override SYS_NI and COMPAT_SYS_NI */
-#include <asm/syscall_wrapper.h>
-#endif
-
-asmlinkage long sys_ni_posix_timers(void)
-{
-	pr_err_once("process %d (%s) attempted a POSIX timer syscall "
-		    "while CONFIG_POSIX_TIMERS is not set\n",
-		    current->pid, current->comm);
-	return -ENOSYS;
-}
-
-#ifndef SYS_NI
-#define SYS_NI(name)  SYSCALL_ALIAS(sys_##name, sys_ni_posix_timers)
-#endif
-
-#ifndef COMPAT_SYS_NI
-#define COMPAT_SYS_NI(name)  SYSCALL_ALIAS(compat_sys_##name, sys_ni_posix_timers)
-#endif
-
-SYS_NI(timer_create);
-SYS_NI(timer_gettime);
-SYS_NI(timer_getoverrun);
-SYS_NI(timer_settime);
-SYS_NI(timer_delete);
-SYS_NI(clock_adjtime);
-SYS_NI(getitimer);
-SYS_NI(setitimer);
-SYS_NI(clock_adjtime32);
-#ifdef __ARCH_WANT_SYS_ALARM
-SYS_NI(alarm);
-#endif
-
 /*
  * We preserve minimal support for CLOCK_REALTIME and CLOCK_MONOTONIC
  * as it is easy to remain compatible with little code. CLOCK_BOOTTIME
@@ -158,18 +124,7 @@ SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
 				 which_clock);
 }
 
-#ifdef CONFIG_COMPAT
-COMPAT_SYS_NI(timer_create);
-#endif
-
-#if defined(CONFIG_COMPAT) || defined(CONFIG_ALPHA)
-COMPAT_SYS_NI(getitimer);
-COMPAT_SYS_NI(setitimer);
-#endif
-
 #ifdef CONFIG_COMPAT_32BIT_TIME
-SYS_NI(timer_settime32);
-SYS_NI(timer_gettime32);
 
 SYSCALL_DEFINE2(clock_settime32, const clockid_t, which_clock,
 		struct old_timespec32 __user *, tp)

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Sami Tolvanen]

On Tue, Dec 19, 2023 at 3:15 PM Linus Torvalds
<torvalds@linuxfoundation.org> wrote:
>
> On Tue, 19 Dec 2023 at 12:17, Linus Torvalds
> <torvalds@linuxfoundation.org> wrote:
> >
> > That said, I still think that just getting rid of this horrid special
> > case for posix timers is the right thing, and we should just remove
> > that SYS_NI() alias thing entirely.
>
> IOW, something like the attached patch.
>
> It's not extensively tested, but hey, the diffstat looks nice:
>
>   arch/arm64/include/asm/syscall_wrapper.h |  4 ---
>   arch/riscv/include/asm/syscall_wrapper.h |  5 ----
>   arch/s390/include/asm/syscall_wrapper.h  | 13 +--------
>   arch/x86/include/asm/syscall_wrapper.h   | 34 +++---------------------
>   kernel/sys_ni.c                          | 14 ++++++++++
>   kernel/time/posix-stubs.c                | 45 --------------------------------
>   6 files changed, 19 insertions(+), 96 deletions(-)
>
> and it builds in at least a *couple* of configurations, including with
> CONFIG_POSIX_TIMERS disabled.
>
> I did *not* check whether it might fix the warning, since I doubt my
> user space would even boot without that posix timer support (actually,
> honestly, because I'm just lazy and "it _looks_ fine to me" was the
> main real thing).

I tested the patch with the 0-day bot reproducer and it does fix the
warning. My usual arm64 and riscv configs also seem to build and boot
just fine.

> But that SYS_NI() thing really does deserve to die, as it was purely
> used as a hack for some random timer system calls.
>
> Comments?

Removing the SYS_NI bits is definitely a cleaner solution. Looks good to me.

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Tested-by: Sami Tolvanen <samitolvanen@google.com>

Sami

Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation

[Linus Torvalds]

On Wed, 20 Dec 2023 at 15:40, Sami Tolvanen <samitolvanen@google.com> wrote:
>
> I tested the patch with the 0-day bot reproducer and it does fix the
> warning. My usual arm64 and riscv configs also seem to build and boot
> just fine.

Thanks. I've been running it on my machine too, and still don't see
anything wrong with it..

I suspect all sane people are already on xmas break, which explains
why people are being quiet. They _should_ be.

But since I'm not in that sane group, I decided to just bypass the
normal channels and apply it directly.

It really isn't all that critical, since I don't expect anybody to
actually disable the posix timer subsystem: I think the config
variable came out of the kernel minimization project, and it's
probably much more likely that people turn off CFI (particularly since
you afaik still need to build with clang to get it) than that they'd
turn off the posix timer support.

But I think it's a worthy cleanup of some messy system call macros, so
I wanted to put this behind us whether it truly matters or not.

            Linus