From: "H. Peter Anvin" <hpa@zytor.com>
To: Sven Wegener <sven.wegener@stealer.net>
Cc: "Richard Weinberger" <richard@nod.at>, "X86 ML" <x86@kernel.org>,
"Eric Paris" <eparis@redhat.com>,
"Linux Kernel" <linux-kernel@vger.kernel.org>,
"Steven Rostedt" <rostedt@goodmis.org>,
"Borislav Petkov" <bp@alien8.de>,
"Toralf Förster" <toralf.foerster@gmx.de>,
stable@vger.kernel.org, "Roland McGrath" <roland@redhat.com>,
"Andy Lutomirski" <luto@amacapital.net>,
"Josh Boyer" <jwboyer@fedoraproject.org>
Subject: Re: [PATCH] x86_32, entry: store badsys error code in %eax
Date: Sun, 20 Jul 2014 15:07:53 -0700 [thread overview]
Message-ID: <90404aa5-cab6-44aa-9eab-4106cb16e7ab@email.android.com> (raw)
In-Reply-To: <alpine.LNX.2.11.1407202330420.31021@titan.int.lan.stealer.net>
This is not a subtle regression at all. It is in fact a very very serious one.
On July 20, 2014 2:33:50 PM PDT, Sven Wegener <sven.wegener@stealer.net> wrote:
>Commit 554086d ("x86_32, entry: Do syscall exit work on badsys
>(CVE-2014-4508)") introduced a subtle regression in the x86_32 syscall
>entry code, resulting in syscall() not returning proper errors for
>non-existing syscalls on CPUs not supporting the sysenter feature.
>
>The following code:
>
>> int result = syscall(666);
>> printf("result=%d errno=%d error=%s\n", result, errno,
>strerror(errno));
>
>results in:
>
>> result=666 errno=0 error=Success
>
>Obviously, the syscall return value is the called syscall number, but
>it
>should have been an ENOSYS error. When run under ptrace it behaves
>correctly, which makes it hard to debug in the wild:
>
>> result=-1 errno=38 error=Function not implemented
>
>The %eax register is the return value register. For debugging via
>ptrace
>the syscall entry code stores the complete register context on the
>stack. The badsys handlers only store the ENOSYS error code in the
>ptrace register set and do not set %eax like a regular syscall handler
>would. The old resume_userspace call chain contains code that clobbers
>%eax and it restores %eax from the ptrace registers afterwards. The
>same
>goes for the ptrace-enabled call chain. When ptrace is not used, the
>syscall return value is the passed-in syscall number from the
>%eax register.
>
>Use %eax as the return value register in syscall_badsys and
>sysenter_badsys, like a real syscall handler does, and have the caller
>push the value onto the stack for ptrace access.
>
>Signed-off-by: Sven Wegener <sven.wegener@stealer.net>
>---
> arch/x86/kernel/entry_32.S | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
>diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
>index dbaa23e..0d0c9d4 100644
>--- a/arch/x86/kernel/entry_32.S
>+++ b/arch/x86/kernel/entry_32.S
>@@ -425,8 +425,8 @@ sysenter_do_call:
> cmpl $(NR_syscalls), %eax
> jae sysenter_badsys
> call *sys_call_table(,%eax,4)
>- movl %eax,PT_EAX(%esp)
> sysenter_after_call:
>+ movl %eax,PT_EAX(%esp)
> LOCKDEP_SYS_EXIT
> DISABLE_INTERRUPTS(CLBR_ANY)
> TRACE_IRQS_OFF
>@@ -502,6 +502,7 @@ ENTRY(system_call)
> jae syscall_badsys
> syscall_call:
> call *sys_call_table(,%eax,4)
>+syscall_after_call:
> movl %eax,PT_EAX(%esp) # store the return value
> syscall_exit:
> LOCKDEP_SYS_EXIT
>@@ -675,12 +676,12 @@ syscall_fault:
> END(syscall_fault)
>
> syscall_badsys:
>- movl $-ENOSYS,PT_EAX(%esp)
>- jmp syscall_exit
>+ movl $-ENOSYS,%eax
>+ jmp syscall_after_call
> END(syscall_badsys)
>
> sysenter_badsys:
>- movl $-ENOSYS,PT_EAX(%esp)
>+ movl $-ENOSYS,%eax
> jmp sysenter_after_call
> END(syscall_badsys)
> CFI_ENDPROC
--
Sent from my mobile phone. Please pardon brevity and lack of formatting.
next prev parent reply other threads:[~2014-07-20 22:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-20 21:33 [PATCH] x86_32, entry: store badsys error code in %eax Sven Wegener
2014-07-20 22:07 ` H. Peter Anvin [this message]
2014-07-21 16:53 ` Andy Lutomirski
2014-07-21 17:20 ` H. Peter Anvin
2014-07-21 17:23 ` Andy Lutomirski
2014-07-21 21:59 ` Andy Lutomirski
2014-07-22 6:57 ` Sven Wegener
2014-07-22 8:26 Sven Wegener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=90404aa5-cab6-44aa-9eab-4106cb16e7ab@email.android.com \
--to=hpa@zytor.com \
--cc=bp@alien8.de \
--cc=eparis@redhat.com \
--cc=jwboyer@fedoraproject.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=richard@nod.at \
--cc=roland@redhat.com \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.org \
--cc=sven.wegener@stealer.net \
--cc=toralf.foerster@gmx.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).