On Sun, Jul 20, 2014 at 3:07 PM, H. Peter Anvin <h...@zytor.com> wrote:
> This is not a subtle regression at all. It is in fact a very very serious
> one.
Indeed.
Does this really work correctly on sysenter? It sure looks like the
sysenter path has the same problem.
Grr. I'm not set up to properly test 32-bit kernels. The patch looks
correct, but I'm obviously not an infallible authority on "looking
correct" when it comes to entry_32.S.
--Andy
>
> On July 20, 2014 2:33:50 PM PDT, Sven Wegener <sven.wege...@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.wege...@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.
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
