linux-kernel - Re: [PATCH 0/8] use struct pt_regs based syscall calling for x86-64

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]

Message-ID: <20180406082322.ud37sj3b5ldm7xxd@gmail.com>
Date:   Fri, 6 Apr 2018 10:23:22 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     linux-kernel@...r.kernel.org, Al Viro <viro@...iv.linux.org.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org
Subject: Re: [PATCH 0/8] use struct pt_regs based syscall calling for x86-64


* Dominik Brodowski <linux@...inikbrodowski.net> wrote:

> On Thu, Apr 05, 2018 at 05:19:33PM +0200, Ingo Molnar wrote:
> > Ok, this series looks mostly good to me, but AFAICS this breaks the UML build:
> > 
> >  make[2]: *** No rule to make target 'archheaders'.  Stop.
> >  arch/um/Makefile:119: recipe for target 'archheaders' failed
> >  make[1]: *** [archheaders] Error 2
> >  make[1]: *** Waiting for unfinished jobs....
> 
> Ah, that's caused by patch 8/8 which I did and do not like all that much
> anyway: UML re-uses syscall_64.tbl which now has x86-specific entries like
> __sys_x86_pread64, but expects the generic syscall stub sys_pread64
> referenced there. Fixup patch below; could be folded with patch 8/8. Or
> patch 8/8 could simply be dropped from the series altogether...

I still like the 'truth in advertising' aspect. For example if I see this in the 
syscall table:

 10      common  mprotect                __sys_x86_mprotect

I can immediately find the _real_ syscall entry point:

ffffffff81180a10 <__sys_x86_mprotect>:
ffffffff81180a10:       48 8b 57 60             mov    0x60(%rdi),%rdx
ffffffff81180a14:       48 8b 77 68             mov    0x68(%rdi),%rsi
ffffffff81180a18:       b9 ff ff ff ff          mov    $0xffffffff,%ecx
ffffffff81180a1d:       48 8b 7f 70             mov    0x70(%rdi),%rdi
ffffffff81180a21:       e8 fa fc ff ff          callq  ffffffff81180720 <do_mprotect_pkey>
ffffffff81180a26:       48 98                   cltq   
ffffffff81180a28:       c3                      retq   
ffffffff81180a29:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

If, on the other hand, I see this entry:

 10     common  mprotect                sys_mprotect

Then, as a first step, no symbol anywhere matches with this:

 triton:~/tip> grep sys_mprotect System.map 
 triton:~/tip> 

"sys_mprotect" does not exist in any easily discoverable sense. You have to *know* 
to replace the sys_ prefix with __sys_x86_ to find it.

Now arguably we could use a __sys_ prefix instead of the grep-barrier __sys_x86 
prefix - but that too would be somewhat confusing I think.

I mean, the fact that we are passing in a ptregs pointer is a complexity of the 
x86 kernel that *exists*, why hide it and make it harder to discover what's 
happening, for something as important as system calls?

In terms of UML breakage, UML arguably is tightly coupled to its host 
architecture:

> Subject: [PATCH] syscalls/x86: fix UML syscall table

Even with your patch applied I still see build failures:

  $ make ARCH=um defconfig
  $ make ARCH=um linux
  ...
  arch/um/os-Linux/signal.c: In function ‘hard_handler’:
  arch/um/os-Linux/signal.c:163:22: error: dereferencing pointer to incomplete type 
  ‘struct ucontext’
    mcontext_t *mc = &uc->uc_mcontext;
                        ^~
  scripts/Makefile.build:324: recipe for target 'arch/um/os-Linux/signal.o' failed
  make[1]: *** [arch/um/os-Linux/signal.o] Error 1

Thanks,

	Ingo