From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39738) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SRMsD-0005No-4r for qemu-devel@nongnu.org; Mon, 07 May 2012 08:17:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SRMs7-0004H8-EY for qemu-devel@nongnu.org; Mon, 07 May 2012 08:17:04 -0400 Received: from e24smtp01.br.ibm.com ([32.104.18.85]:33089) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SRMs7-0004GP-3Y for qemu-devel@nongnu.org; Mon, 07 May 2012 08:16:59 -0400 Received: from /spool/local by e24smtp01.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 May 2012 09:16:52 -0300 Received: from d24relay02.br.ibm.com (d24relay02.br.ibm.com [9.13.184.26]) by d24dlp01.br.ibm.com (Postfix) with ESMTP id 12E6A352004C for ; Mon, 7 May 2012 09:16:32 -0300 (BRT) Received: from d24av04.br.ibm.com (d24av04.br.ibm.com [9.8.31.97]) by d24relay02.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q47CGBXG34734536 for ; Mon, 7 May 2012 09:16:12 -0300 Received: from d24av04.br.ibm.com (loopback [127.0.0.1]) by d24av04.br.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q47AGMtU009088 for ; Mon, 7 May 2012 07:16:22 -0300 Date: Mon, 7 May 2012 09:16:34 -0300 From: Eduardo Otubo Message-ID: <20120507121633.GB10516@bluepex.com> References: <4FA45124.4050207@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4FA45124.4050207@suse.de> Subject: Re: [Qemu-devel] [RFC] [PATCH 2/2] Adding basic calls to libseccomp in vl.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: qemu-devel@nongnu.org On Fri, May 04, 2012 at 11:59:00PM +0200, Andreas Färber wrote: > 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 > > --- > > 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 > > #include > > +#ifdef CONFIG_LIBSECCOMP > > +#include > > +#endif > > #endif > > #ifdef __sun__ > > #include > > @@ -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... Actually I did run the scripts/checkpatch.pl, and there were no errors. But I'll fix those issues for my next version. Thanks for pointing them out :) > > > + > > 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. > Agreed. > > + } > > +#endif > > This is adding code before the variable declaration block, please move > to after. Agreed. Thanks for the comments. -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 eotubo@linux.vnet.ibm.com