[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC] [PATCH 2/2] Adding basic calls to libseccomp in v
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [RFC] [PATCH 2/2] Adding basic calls to libseccomp in vl.c |
Date: |
Fri, 04 May 2012 23:59:00 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
Am 04.05.2012 21:08, schrieb Eduardo Otubo:
> I added a syscall struct using priority levels as described in the libseccomp
> man page. The priority numbers are based to the frequency they appear in a
> sample strace from a regular qemu guest run under libvirt.
>
> Libseccomp generates linear BPF code to filter system calls, those rules are
> read one after another. The priority system places the most common rules first
> in order to reduce the overhead when processing them.
>
> Also, since this is just a first RFC, the whitelist is a little raw. We might
> need your help to improve, test and fine tune the set of system calls.
>
> Signed-off-by: Eduardo Otubo <address@hidden>
> ---
> vl.c | 81
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 81 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index ae91a8a..e23838b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -63,6 +63,9 @@
>
> #include <linux/ppdev.h>
> #include <linux/parport.h>
> +#ifdef CONFIG_LIBSECCOMP
> +#include <seccomp.h>
> +#endif
> #endif
> #ifdef __sun__
> #include <sys/stat.h>
> @@ -175,6 +178,8 @@ int main(int argc, char **argv)
>
> #define MAX_VIRTIO_CONSOLES 1
>
> +static int seccomp_start(void);
You haven't tested this without libseccomp apparently?
Other than that mostly some Coding Style issues...
> +
> static const char *data_dir;
> const char *bios_name = NULL;
> enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
> @@ -313,6 +318,75 @@ static int default_driver_check(QemuOpts *opts, void
> *opaque)
> return 0;
> }
>
> +#ifdef CONFIG_LIBSECCOMP
> +struct qemu_seccomp_syscall {
Struct names are expected to be CamelCase.
> + int32_t num;
> + uint8_t priority;
> +};
> +
> +static struct qemu_seccomp_syscall seccomp_whitelist[] = {
> + {SCMP_SYS(timer_settime), 255},
Spaces inside braces please.
> + {SCMP_SYS(timer_gettime), 254},
> + {SCMP_SYS(futex), 253},
> + {SCMP_SYS(select), 252},
> + {SCMP_SYS(recvfrom), 251},
> + {SCMP_SYS(sendto), 250},
> + {SCMP_SYS(read), 249},
> + {SCMP_SYS(brk), 248},
> + {SCMP_SYS(clone), 247},
> + {SCMP_SYS(mmap), 247},
> + {SCMP_SYS(mprotect), 246},
> + {SCMP_SYS(rt_sigprocmask), 245},
> + {SCMP_SYS(write), 244},
> + {SCMP_SYS(fcntl), 243},
> + {SCMP_SYS(tgkill), 242},
> + {SCMP_SYS(rt_sigaction), 242},
> + {SCMP_SYS(pipe2), 242},
> + {SCMP_SYS(munmap), 242},
> + {SCMP_SYS(mremap), 242},
> + {SCMP_SYS(getsockname), 242},
> + {SCMP_SYS(getpeername), 242},
> + {SCMP_SYS(fdatasync), 242},
> + {SCMP_SYS(close), 242}
> +};
> +
> +#define seccomp_whitelist_count \
> + (sizeof(seccomp_whitelist)/sizeof(seccomp_whitelist[0]))
Please just use the ARRAY_SIZE() macro inline.
> +
> +int seccomp_start(void)
> +{
> + int rc = 0;
> + unsigned int i = 0;
> +
> + rc = seccomp_init(SCMP_ACT_KILL);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> +
> + for (i = 0; i < seccomp_whitelist_count; i++) {
> + rc = seccomp_rule_add(SCMP_ACT_ALLOW, seccomp_whitelist[i].num, 0);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + rc = seccomp_syscall_priority(seccomp_whitelist[i].num,
> + seccomp_whitelist[i].priority);
> + if (rc < 0) {
> + goto seccomp_return;
> + }
> + }
> +
> + rc = seccomp_load();
> +
> + seccomp_return:
> + seccomp_release();
> + if (rc < 0) {
> + fprintf(stderr,
> + "ERROR: failed to configure the seccomp syscall filter in
> the kernel");
\n missing.
> + }
> + return rc;
> +}
> +#endif
> +
> /***********************************************************/
> /* QEMU state */
>
> @@ -2257,6 +2331,13 @@ int qemu_init_main_loop(void)
>
> int main(int argc, char **argv, char **envp)
> {
> +
> +#ifdef CONFIG_LIBSECCOMP
> + if (seccomp_start() < 0) {
> + exit(1);
This is inconsistent: Either exit() within seccomp_start() where you
print the error message, or move the fprintf() here.
> + }
> +#endif
This is adding code before the variable declaration block, please move
to after.
> +
> int i;
> int snapshot, linux_boot;
> const char *icount_option = NULL;
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [RFC] [PATCH 0/2] Sandboxing Qemu guests with Libseccomp, Daniel P. Berrange, 2012/05/08