From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753554AbbDBPUh (ORCPT ); Thu, 2 Apr 2015 11:20:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54032 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753355AbbDBPUe (ORCPT ); Thu, 2 Apr 2015 11:20:34 -0400 Message-ID: <551D5E23.2050002@redhat.com> Date: Thu, 02 Apr 2015 17:20:03 +0200 From: Denys Vlasenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Brian Gerst CC: Ingo Molnar , Linus Torvalds , Steven Rostedt , Borislav Petkov , "H. Peter Anvin" , Andy Lutomirski , Oleg Nesterov , Frederic Weisbecker , Alexei Starovoitov , Will Drewry , Kees Cook , the arch/x86 maintainers , Linux Kernel Mailing List Subject: Re: [PATCH 3/9] x86/asm/entry/64: do not SAVE_EXTRA_REGS in stub_sigreturn References: <1427985378-4287-1-git-send-email-dvlasenk@redhat.com> <1427985378-4287-3-git-send-email-dvlasenk@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/02/2015 05:01 PM, Brian Gerst wrote: > On Thu, Apr 2, 2015 at 10:36 AM, Denys Vlasenko wrote: >> stub_sigreturn ignores old values of pt_regs->REG for all general-purpose >> registers, it sets them to values saved on userspace >> signal stack. >> >> Which is hardly surprising - it would be a bug if it would use pt_regs->REG. >> sigreturn must restore all registers. >> >> Therefore, SAVE_EXTRA_REGS in it ought to be redundant. >> >> It is a leftover from the time SAVE_EXTRA_REGS wasn't only saving registers, >> but it also was extending stack to "full" pt_regs. >> >> Delete this SAVE_EXTRA_REGS. >> >> Run-tested. >> >> Signed-off-by: Denys Vlasenko >> CC: Linus Torvalds >> CC: Steven Rostedt >> CC: Ingo Molnar >> CC: Borislav Petkov >> CC: "H. Peter Anvin" >> CC: Andy Lutomirski >> CC: Oleg Nesterov >> CC: Frederic Weisbecker >> CC: Alexei Starovoitov >> CC: Will Drewry >> CC: Kees Cook >> CC: x86@kernel.org >> CC: linux-kernel@vger.kernel.org >> --- >> arch/x86/kernel/entry_64.S | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S >> index ec51598..1cf245d 100644 >> --- a/arch/x86/kernel/entry_64.S >> +++ b/arch/x86/kernel/entry_64.S >> @@ -447,7 +447,12 @@ ENTRY(stub_rt_sigreturn) >> CFI_STARTPROC >> addq $8, %rsp >> DEFAULT_FRAME 0 >> - SAVE_EXTRA_REGS >> + /* >> + * Despite RESTORE_EXTRA_REGS in return_from_stub, >> + * no need to SAVE_EXTRA_REGS here: >> + * sys_rt_sigreturn overwrites all general purpose pt_regs->REGs >> + * on stack, for RESTORE_{EXTRA,C}_REGS to pick them up. >> + */ >> call sys_rt_sigreturn >> jmp return_from_stub >> CFI_ENDPROC >> @@ -458,7 +463,7 @@ ENTRY(stub_x32_rt_sigreturn) >> CFI_STARTPROC >> addq $8, %rsp >> DEFAULT_FRAME 0 >> - SAVE_EXTRA_REGS >> + /* No need to SAVE_EXTRA_REGS */ >> call sys32_x32_rt_sigreturn >> jmp return_from_stub >> CFI_ENDPROC > > I had the same idea, but determined sigreturn can fault and return an > error code without modifying all the registers. This would leak junk > from the stack. This still can be made to work by not RESTORE'ing EXTRA_REGS either, if there is a way to detect the failure: call sys_rt_sigreturn - jmp return_from_stub + testl ??????????? + jz return_from_stub + ret CFI_ENDPROC But this is not a normal syscall, off-hand I don't see an easy way to do the test. sys_rt_sigreturn() on failure runs this code: ... segfault: force_sig(SIGSEGV, current); return 0; } Help?