[PATCH RFC v2 1/3] Introduce seccomp-assisted syscall filtering

[Dmitry V. Levin]

On Wed, Jul 31, 2019 at 05:35:56PM +0200, Paul Chaignon wrote:
[...]
> diff --git a/defs.h b/defs.h
> index 51622c05..1ff17ae8 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -147,6 +147,15 @@ extern char *stpcpy(char *dst, const char *src);
>  #  define HAVE_PERSONALITY_2_MPERS 0
>  # endif
>  
> +# ifndef PERSONALITY0_AUDIT_ARCH
> +#  define PERSONALITY0_AUDIT_ARCH  { 0, 0 }
> +# endif
> +# if SUPPORTED_PERSONALITIES > 1
> +#  ifndef PERSONALITY1_AUDIT_ARCH
> +#   define PERSONALITY1_AUDIT_ARCH { 0, 0 }
> +#  endif
> +# endif
> +

Do you plan to use these macros outside filter_seccomp.c?
If not, why do you make them available globally by defining in defs.h
file?

> --- /dev/null
> +++ b/filter_seccomp.c
> @@ -0,0 +1,467 @@
> +/*
> + * Copyright (c) 2018 Chen Jingpiao <chenjingpiao at gmail.com>
> + * Copyright (c) 2018 The strace developers.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + *    derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "defs.h"
> +
> +#include "ptrace.h"
> +#include <sys/prctl.h>
> +#include <sys/wait.h>
> +#include <linux/audit.h>
> +#include <linux/filter.h>
> +#include <linux/seccomp.h>
> +#include <asm/unistd.h>
> +#include <signal.h>
> +
> +#include "filter_seccomp.h"
> +#include "number_set.h"
> +#include "syscall.h"
> +
> +#define JMP_PLACEHOLDER_NEXT  ((unsigned char) -1)
> +#define JMP_PLACEHOLDER_TRACE ((unsigned char) -2)
> +
> +#define SET_BPF(filter, code, jt, jf, k) \
> +	(*(filter) = (struct sock_filter) { code, jt, jf, k })
> +
> +#define SET_BPF_STMT(filter, code, k) \
> +	SET_BPF(filter, code, 0, 0, k)
> +
> +#define SET_BPF_JUMP(filter, code, k, jt, jf) \
> +	SET_BPF(filter, code, jt, jf, k)
> +
> +struct audit_arch_t {
> +	unsigned int arch;
> +	unsigned int mask;
> +};
> +
> +struct audit_arch_t audit_arch_vec[SUPPORTED_PERSONALITIES] = {

Please declare audit_arch_vec and other similar objects with the file
scope using static qualifier.

> +	PERSONALITY0_AUDIT_ARCH,
> +#if SUPPORTED_PERSONALITIES > 1
> +	PERSONALITY1_AUDIT_ARCH,
> +# if SUPPORTED_PERSONALITIES > 2
> +	PERSONALITY2_AUDIT_ARCH,
> +# endif
> +#endif
> +};

I was thinking of something like

static struct audit_arch_t audit_arch_vec[SUPPORTED_PERSONALITIES] = {
#ifdef PERSONALITY0_AUDIT_ARCH
[0] = PERSONALITY0_AUDIT_ARCH,
#endif
#ifdef PERSONALITY1_AUDIT_ARCH
[1] = PERSONALITY1_AUDIT_ARCH,
#endif
#ifdef PERSONALITY2_AUDIT_ARCH
[2] = PERSONALITY2_AUDIT_ARCH,
#endif
};

This way you don't need any fallback PERSONALITY{0,1,2}_AUDIT_ARCH macros.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20190805/44cf055b/attachment.bin>