linux-kernel - Re: [Regression v4.2 ?] 32-bit seccomp-BPF returned errno values wrong in VM?

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]

Date:	Thu, 13 Aug 2015 15:27:26 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Denys Vlasenko <dvlasenk@...hat.com>
Cc:	Kees Cook <keescook@...omium.org>,
	David Drysdale <drysdale@...gle.com>,
	Andy Lutomirski <luto@...capital.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Will Drewry <wad@...omium.org>, Ingo Molnar <mingo@...nel.org>,
	Alok Kataria <akataria@...are.com>,
	Borislav Petkov <bp@...en8.de>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	"H. Peter Anvin" <hpa@...or.com>, Oleg Nesterov <oleg@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>, X86 ML <x86@...nel.org>
Subject: Re: [Regression v4.2 ?] 32-bit seccomp-BPF returned errno values
 wrong in VM?

On Thu, Aug 13, 2015 at 2:35 PM, Denys Vlasenko <dvlasenk@...hat.com> 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
--
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/