linux-kernel - Re: [PATCH] x86/asm/entry/32: Update ENOSYS handling to match 64-bit logic

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: <5527F576.8030709@redhat.com>
Date:	Fri, 10 Apr 2015 18:08:22 +0200
From:	Denys Vlasenko <dvlasenk@...hat.com>
To:	Andy Lutomirski <luto@...capital.net>
CC:	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>, Oleg Nesterov <oleg@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Will Drewry <wad@...omium.org>,
	Kees Cook <keescook@...omium.org>, X86 ML <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/asm/entry/32: Update ENOSYS handling to match 64-bit
 logic

On 04/10/2015 05:44 PM, Andy Lutomirski wrote:
> On Fri, Apr 10, 2015 at 8:33 AM, Denys Vlasenko <dvlasenk@...hat.com> wrote:
>> Sometime ago Andy changed 64-bit syscall logic so that pt_regs->ax is
>> initially set to -ENOSYS, and on exit from syscall, it is updated with
>> actual return value. This simplified logic there.
>>
>> This patch does the same for 32-bit syscall entry points.
>>
>> The check for %rax being too big is moved to be just before
>> the call insn which dispatches execution through syscall table.
>> There is no way to accidentally skip this check now by jumping
>> to a label after it. This allows to remove redundant checks
>> after e.g. ptrace.
>>
>> If %rax is too big, we just skip over the (call, write %rax to pt_regs->ax)
>> insn pair. pt_regs->ax remains set to -ENOSYS, and it gets returned
>> to userspace.
> 
> This looks okay, but I'll read it again later today.

Unfortunately, there is a mistake.

>> @@ -247,9 +248,7 @@ sysexit_from_sys_call:
>>         movl %ebx,%esi                  /* 2nd arg: 1st syscall arg */
>>         movl %eax,%edi                  /* 1st arg: syscall number */
>>         call __audit_syscall_entry
>> -       movl RAX(%rsp),%eax     /* reload syscall number */
>> -       cmpq $(IA32_NR_syscalls-1),%rax
>> -       ja ia32_badsys
>> +       movl ORIG_RAX(%rsp),%eax        /* reload syscall number */

this is correct, now syscall# is only in orig_ax, not in ax...

>>         call __audit_syscall_exit
>> -       movq RAX(%rsp),%rax     /* reload syscall return value */
>> +       movq ORIG_RAX(%rsp),%rax        /* reload syscall return value */

but here I'm wrong, this line should not be changed.
Will send v2 now.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/