From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754287AbbHMW12 (ORCPT ); Thu, 13 Aug 2015 18:27:28 -0400 Received: from mail-ig0-f171.google.com ([209.85.213.171]:35468 "EHLO mail-ig0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754135AbbHMW11 (ORCPT ); Thu, 13 Aug 2015 18:27:27 -0400 MIME-Version: 1.0 In-Reply-To: <55CD0DAC.9080809@redhat.com> References: <55CCB510.3060807@redhat.com> <55CD0DAC.9080809@redhat.com> Date: Thu, 13 Aug 2015 15:27:26 -0700 X-Google-Sender-Auth: yVLJmrIGepf1TaLM-Fvc4h7CYTc Message-ID: Subject: Re: [Regression v4.2 ?] 32-bit seccomp-BPF returned errno values wrong in VM? From: Linus Torvalds To: Denys Vlasenko Cc: Kees Cook , David Drysdale , Andy Lutomirski , "linux-kernel@vger.kernel.org" , Will Drewry , Ingo Molnar , Alok Kataria , Borislav Petkov , Alexei Starovoitov , Frederic Weisbecker , "H. Peter Anvin" , Oleg Nesterov , Steven Rostedt , X86 ML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 13, 2015 at 2:35 PM, Denys Vlasenko wrote: > > I suspect this change: > > .macro auditsys_entry_common > ... > 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 */ > > We were reloading syscall# from pt_regs->ax. > > After the patch, pt_regs->ax isn't equal to syscall# on entry, > instead it contains -ENOSYS. Therefore the change shown above > was made, to reload it from pt_regs->orig_ax. > > Well. This still should work... in fact it is "more correct" > than it was before... Well, since it doesn't work, that's clearly not the case. Also, you do realize that ORIG_RAX can get changed by signal handling and ptrace? In fact, I think that whole "save -ENOSYS to pt_regs->ax" is BS. Exactly because we use pt_regs->ax for ptrace etc, and you've changed the register state we expose. So, considering that we're in -rc6, and this has been bisected, I would normally just revert the commit with extreme prejudice. However, in this case it's unnecessarily painful, just because there's been a ton of pointless churn in this area (cfi annotations, renaming, no end of crap. I'm also going to be a *lot* less inclined to take all these idiotic low-level x86 changes from now on. It's been a total pain, for very little gain. These "cleanups" have been buggy as hell, and test coverage for the compat case is clearly lacking. Denys, this was not the first "obvious cleanup" of yours that broke things subtly, and where your reaction to the problem report was "that cannot happen". Linus