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

From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Borislav Petkov <bp@alien8.de>
Cc: kernel test robot <oliver.sang@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	oe-lkp@lists.linux.dev,  lkp@intel.com,
	linux-kernel@vger.kernel.org,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [linus:master] [x86/entry] be5341eb0d: WARNING:CPU:#PID:#at_int80_emulation
Date: Tue, 19 Dec 2023 10:20:58 -0800	[thread overview]
Message-ID: <CAHk-=wimbX8UF6ECo53Hm4Vz0tCC7jjN9e3tEhZfoEtsxyfU-A@mail.gmail.com> (raw)
In-Reply-To: <20231219095821.GAZYFpPUSKexZAcl05@fat_crate.local>

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