[v3] Add argument filters to the seccomp sandbox

Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.

Changes so far:
v1
 - Created argument filters for the madvise, shmget, and shmctl syscalls.
v1 -> v2
 - Added 5 new madvise flags which were present in the source code but not in
   the strace which I generated.
 - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
   present in GTK2, which QEMU uses but does not call directly.
v2 -> v3
 - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
 - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
 - Removed the comma on the last entry of the madvise_flags array.
 - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.

Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
---

On Fri, Sep 25, 2015 at 12=50=36AM -0400, Namsun Ch'o wrote:
> Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.
> 
> Changes so far:
> v1
>  - Created argument filters for the madvise, shmget, and shmctl syscalls.
> v1 -> v2
>  - Added 5 new madvise flags which were present in the source code but not in
>    the strace which I generated.
>  - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
>    present in GTK2, which QEMU uses but does not call directly.
> v2 -> v3
>  - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
>  - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
>  - Removed the comma on the last entry of the madvise_flags array.
>  - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.
> 
> Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
> ---
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index f9de0d3..a353ef9 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -14,6 +14,8 @@
>   */
>  #include <stdio.h>
>  #include <seccomp.h>
> +#include <linux/ipc.h>
> +#include <sys/mman.h>
>  #include "sysemu/seccomp.h"
>  
>  struct QemuSeccompSyscall {
> @@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
>      { SCMP_SYS(rt_sigreturn), 245 },
>      { SCMP_SYS(sync), 245 },
>      { SCMP_SYS(pread64), 245 },
> -    { SCMP_SYS(madvise), 245 },
>      { SCMP_SYS(set_robust_list), 245 },
>      { SCMP_SYS(lseek), 245 },
>      { SCMP_SYS(pselect6), 245 },
> @@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
>      { SCMP_SYS(arch_prctl), 240 },
>      { SCMP_SYS(mkdir), 240 },
>      { SCMP_SYS(fchmod), 240 },
> -    { SCMP_SYS(shmget), 240 },
>      { SCMP_SYS(shmat), 240 },
>      { SCMP_SYS(shmdt), 240 },
>      { SCMP_SYS(timerfd_create), 240 },
> -    { SCMP_SYS(shmctl), 240 },
>      { SCMP_SYS(mlockall), 240 },
>      { SCMP_SYS(mlock), 240 },
>      { SCMP_SYS(munlock), 240 },
> @@ -264,6 +263,59 @@ int seccomp_start(void)
>          }
>      }
>  
> +    /* madvise */
> +    static const int madvise_flags[] = {
> +        MADV_DODUMP,
> +        MADV_DONTDUMP,
> +        MADV_UNMERGEABLE,
> +        MADV_WILLNEED,
> +        MADV_DONTFORK,
> +        MADV_DONTNEED,
> +        MADV_HUGEPAGE,
> +        MADV_MERGEABLE
> +    };
> +    for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
> +        rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
> +            SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
> +        if (rc < 0) {
> +            goto seccomp_return;
> +        }
> +    }
> +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +
> +    /* shmget */
> +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> +        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> +        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> +        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> +        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +
> +    /* shmctl */
> +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
> +        SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
> +        SCMP_A2(SCMP_CMP_EQ, 0));
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
> +    if (rc < 0) {
> +        goto seccomp_return;
> +    }
> +
>      rc = seccomp_load(ctx);
>  
>    seccomp_return:

This looks good now.
Thanks for your contribution.

Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>

ps.: I'll create a pull request with all changes made so far on Friday.

On Tue, Sep 29, 2015 at 05=22=44PM +0200, Eduardo Otubo wrote:
> On Fri, Sep 25, 2015 at 12=50=36AM -0400, Namsun Ch'o wrote:
> > Here's the v3 patch. I applied it and compiled QEMU, and it worked fine.
> > 
> > Changes so far:
> > v1
> >  - Created argument filters for the madvise, shmget, and shmctl syscalls.
> > v1 -> v2
> >  - Added 5 new madvise flags which were present in the source code but not in
> >    the strace which I generated.
> >  - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
> >    present in GTK2, which QEMU uses but does not call directly.
> > v2 -> v3
> >  - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
> >  - Fixed a stupid typo where I had IP_CREAT instead of IPC_CREAT.
> >  - Removed the comma on the last entry of the madvise_flags array.
> >  - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.
> > 
> > Signed-off-by: Namsun Ch'o <namnamc@safe-mail.net>
> > ---
> > diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> > index f9de0d3..a353ef9 100644
> > --- a/qemu-seccomp.c
> > +++ b/qemu-seccomp.c
> > @@ -14,6 +14,8 @@
> >   */
> >  #include <stdio.h>
> >  #include <seccomp.h>
> > +#include <linux/ipc.h>
> > +#include <sys/mman.h>
> >  #include "sysemu/seccomp.h"
> >  
> >  struct QemuSeccompSyscall {
> > @@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> >      { SCMP_SYS(rt_sigreturn), 245 },
> >      { SCMP_SYS(sync), 245 },
> >      { SCMP_SYS(pread64), 245 },
> > -    { SCMP_SYS(madvise), 245 },
> >      { SCMP_SYS(set_robust_list), 245 },
> >      { SCMP_SYS(lseek), 245 },
> >      { SCMP_SYS(pselect6), 245 },
> > @@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = {
> >      { SCMP_SYS(arch_prctl), 240 },
> >      { SCMP_SYS(mkdir), 240 },
> >      { SCMP_SYS(fchmod), 240 },
> > -    { SCMP_SYS(shmget), 240 },
> >      { SCMP_SYS(shmat), 240 },
> >      { SCMP_SYS(shmdt), 240 },
> >      { SCMP_SYS(timerfd_create), 240 },
> > -    { SCMP_SYS(shmctl), 240 },
> >      { SCMP_SYS(mlockall), 240 },
> >      { SCMP_SYS(mlock), 240 },
> >      { SCMP_SYS(munlock), 240 },
> > @@ -264,6 +263,59 @@ int seccomp_start(void)
> >          }
> >      }
> >  
> > +    /* madvise */
> > +    static const int madvise_flags[] = {
> > +        MADV_DODUMP,
> > +        MADV_DONTDUMP,
> > +        MADV_UNMERGEABLE,
> > +        MADV_WILLNEED,
> > +        MADV_DONTFORK,
> > +        MADV_DONTNEED,
> > +        MADV_HUGEPAGE,
> > +        MADV_MERGEABLE
> > +    };
> > +    for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
> > +        rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
> > +            SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
> > +        if (rc < 0) {
> > +            goto seccomp_return;
> > +        }
> > +    }
> > +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +
> > +    /* shmget */
> > +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> > +        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> > +        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
> > +        SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
> > +        SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +
> > +    /* shmctl */
> > +    rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
> > +        SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
> > +        SCMP_A2(SCMP_CMP_EQ, 0));
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +    rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
> > +    if (rc < 0) {
> > +        goto seccomp_return;
> > +    }
> > +
> >      rc = seccomp_load(ctx);
> >  
> >    seccomp_return:
> 
> This looks good now.
> Thanks for your contribution.
> 
> Acked-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> 
> ps.: I'll create a pull request with all changes made so far on Friday.
> 

The pull request will be delayed a little bit due to some new patches
incoming. Let's just set an agreement on how to approach regarding the
"-runas and -chroot" patch and will prepare just a single batch for pull
reuqest to Peter.

Regards,