[PATCH v2 2/3] x86/entry: Fix entry/exit mismatch on failed fast 32-bit syscalls

[PATCH v2 2/3] x86/entry: Fix entry/exit mismatch on failed fast 32-bit syscalls

[Andy Lutomirski]

On a 32-bit fast syscall that fails to read its arguments from user
memory, the kernel currently does syscall exit work but not
syscall entry work.  This confuses audit and ptrace.  For example:

    $ ./tools/testing/selftests/x86/syscall_arg_fault_32
    ...
    strace: pid 264258: entering, ptrace_syscall_info.op == 2
    ...

This is a minimal fix intended for ease of backporting.  A more
complete cleanup is coming.

Cc: stable@vger.kernel.org
Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry")
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 0904f5676e4d..cf4dcf346ca8 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs)
 		regs->ax = -EFAULT;
 
 		instrumentation_end();
-		syscall_exit_to_user_mode(regs);
+		local_irq_disable();
+		exit_to_user_mode();
 		return false;
 	}
 
-- 
2.29.2

Re: [PATCH v2 2/3] x86/entry: Fix entry/exit mismatch on failed fast 32-bit syscalls

[Thomas Gleixner]

On Tue, Feb 23 2021 at 10:15, Andy Lutomirski wrote:
> On a 32-bit fast syscall that fails to read its arguments from user
> memory, the kernel currently does syscall exit work but not
> syscall entry work.  This confuses audit and ptrace.  For example:
>
>     $ ./tools/testing/selftests/x86/syscall_arg_fault_32
>     ...
>     strace: pid 264258: entering, ptrace_syscall_info.op == 2
>     ...
>
> This is a minimal fix intended for ease of backporting.  A more
> complete cleanup is coming.
>
> Cc: stable@vger.kernel.org
> Fixes: 0b085e68f407 ("x86/entry: Consolidate 32/64 bit syscall entry")
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/entry/common.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 0904f5676e4d..cf4dcf346ca8 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -128,7 +128,8 @@ static noinstr bool __do_fast_syscall_32(struct pt_regs *regs)
>  		regs->ax = -EFAULT;
>  
>  		instrumentation_end();
> -		syscall_exit_to_user_mode(regs);
> +		local_irq_disable();
> +		exit_to_user_mode();

That's still the same as the previous version. The right function (while
the name is misleading) to invoke here is irqentry_exit_to_user_mode()
because that invokes exit_to_user_mode_prepare() before
exit_to_user_mode(). We can rename that function afterwards.

Thanks,

        tglx