[PATCH v3 4/4] tests: test cases for seccomp-assisted syscall filtering

[Paul Chaignon]

On Fri, Aug 23, 2019 at 06:17:28PM +0300, Dmitry V. Levin wrote:
> On Thu, Aug 15, 2019 at 07:52:54PM +0200, Paul Chaignon wrote:
> [...]
> > diff --git a/tests/filter_seccomp-perf.c b/tests/filter_seccomp-perf.c
> > new file mode 100644
> > index 00000000..dbc8dcd7
> > --- /dev/null
> > +++ b/tests/filter_seccomp-perf.c
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Check seccomp-filter is enabled.
> > + *
> > + * Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "tests.h"
> > +#include <signal.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +
> > +static volatile bool stop = false;
> > +
> > +static void
> > +handler(int signo) {
> > +    stop = true;
> > +}
> > +
> > +int main(void) {
> > +	signal(SIGINT, handler);
> > +	unsigned int i = 0;
> > +	long rc = 0;
> > +	while (!stop) {
> > +		rc &= chdir(".");
> > +		i++;
> > +	}
> > +	printf("%d\n", i);
> > +	return rc;
> > +}
> 
> chdir returns int, and rc is always 0 in this test,
> did you mean something different?
> 
> A for loop might look simpler, e.g.
> 
> for (i = 0; !stop; ++i)

That whole test was a mess.  I've committed a new version.

> 
> > diff --git a/tests/filter_seccomp-perf.test b/tests/filter_seccomp-perf.test
> > new file mode 100755
> > index 00000000..eb60c107
> > --- /dev/null
> > +++ b/tests/filter_seccomp-perf.test
> > @@ -0,0 +1,17 @@
> > +#!/bin/sh
> > +#
> > +# Check seccomp-filter is enabled.
> > +#
> > +# Copyright (c) 2019 Paul Chaignon <paul.chaignon at gmail.com>
> > +# All rights reserved.
> > +#
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +. "${srcdir=.}/init.sh"
> > +
> > +nb_no_seccomp=$(timeout -sINT 1s $STRACE -qf -e signal=none -efchdir ../$NAME)
> > +nb_seccomp=$(timeout -sINT 1s $STRACE -qfn -e signal=none -efchdir ../$NAME 2> "$OUT")
> 
> Wouldn't alarm(2) be simpler than timeout(1)?
> 
> > +grep "seccomp-filter is requested but unavailable" "$OUT" > /dev/null
> 
> I suppose the test should be skipped if seccomp filtering is unavailable.

Do you mean it should implement the same prctl(PR_SET_SECCOMP,
SECCOMP_MODE_FILTER) + NOMMU_SYSTEM checks as check_seccomp_filter()?

> 
> > +if [ $? -ne 0 ] && [ "$nb_seccomp" -lt "$((10*nb_no_seccomp))" ]; then
> > +	fail_ "Failed to enable seccomp-filter"
> > +fi
> 
> Why 10?

It's mostly an arbitrary number.  On my system, filter_seccomp-perf
performs about 17-18x more chdir syscalls when seccomp-filter is enabled.
So using 10 should give us a little leeway.  I'll add a small comment.

Paul