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

From: Linus Torvalds
Date: Thu Aug 13 2015 - 18:27:31 EST


On Thu, Aug 13, 2015 at 2:35 PM, Denys Vlasenko <dvlasenk@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/