• Daniel Borkmann's avatar
    seccomp: fix populating a0-a5 syscall args in 32-bit x86 BPF · 2eac7648
    Daniel Borkmann authored
    Linus reports that on 32-bit x86 Chromium throws the following seccomp
    resp. audit log messages:
    
      audit: type=1326 audit(1397359304.356:28108): auid=500 uid=500
    gid=500 ses=2 subj=unconfined_u:unconfined_r:chrome_sandbox_t:s0-s0:c0.c1023
    pid=3677 comm="chrome" exe="/opt/google/chrome/chrome" sig=0
    syscall=172 compat=0 ip=0xb2dd9852 code=0x30000
    
      audit: type=1326 audit(1397359304.356:28109): auid=500 uid=500
    gid=500 ses=2 subj=unconfined_u:unconfined_r:chrome_sandbox_t:s0-s0:c0.c1023
    pid=3677 comm="chrome" exe="/opt/google/chrome/chrome" sig=0 syscall=5
    compat=0 ip=0xb2dd9852 code=0x50000
    
    These audit messages are being triggered via audit_seccomp() through
    __secure_computing() in seccomp mode (BPF) filter with seccomp return
    codes 0x30000 (== SECCOMP_RET_TRAP) and 0x50000 (== SECCOMP_RET_ERRNO)
    during filter runtime. Moreover, Linus reports that x86_64 Chromium
    seems fine.
    
    The underlying issue that explains this is that the implementation of
    populate_seccomp_data() is wrong. Our seccomp data structure sd that
    is being shared with user ABI is:
    
      struct seccomp_data {
        int nr;
        __u32 arch;
        __u64 instruction_pointer;
        __u64 args[6];
      };
    
    Therefore, a simple cast to 'unsigned long *' for storing the value of
    the syscall argument via syscall_get_arguments() is just wrong as on
    32-bit x86 (or any other 32bit arch), it would result in storing a0-a5
    at wrong offsets in args[] member, and thus i) could leak stack memory
    to user space and ii) tampers with the logic of seccomp BPF programs
    that read out and check for syscall arguments:
    
      syscall_get_arguments(task, regs, 0, 1, (unsigned long *) &sd->args[0]);
    
    Tested on 32-bit x86 with Google Chrome, unfortunately only via remote
    test machine through slow ssh X forwarding, but it fixes the issue on
    my side. So fix it up by storing args in type correct variables, gcc
    is clever and optimizes the copy away in other cases, e.g. x86_64.
    
    Fixes: bd4cf0ed ("net: filter: rework/optimize internal BPF interpreter's instruction set")
    Reported-and-bisected-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    Signed-off-by: default avatarDaniel Borkmann <dborkman@redhat.com>
    Signed-off-by: default avatarAlexei Starovoitov <ast@plumgrid.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Eric Paris <eparis@redhat.com>
    Cc: James Morris <james.l.morris@oracle.com>
    Cc: Kees Cook <keescook@chromium.org>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    2eac7648
seccomp.c 13.4 KB