[PATCH v3 2/6] Introduce new filtering architecture

Eugene Syromiatnikov esyr at redhat.com
Mon Jul 3 12:02:24 UTC 2017


On Thu, Jun 29, 2017 at 02:46:11PM +0700, Nikolay Marchuk wrote:
> This change introduces new filtering architecture primitives: filter,
> filter_action and bool_expression. It also changes processing of -e
> strace option. Now, filtering is done after decoding of syscall and
> tcp->qual_flg stores filtering results.
> 
> * basic_actions.c: New file.
> * basic_filters.c: Rename functions, add new filters' functions.
> * defs.h (struct tcb): Change inject options variable.
> (QUAL_READ, QUAL_WRITE): Change description.
> (read_set, write_set): Remove global set variables.
> (is_number_in_set): Change declaration.
> (qualify, qual_flags): Remove old declarations ...
> (filter_syscall, parse_qualify_filter,
>  sort_filter_actions): ... and add new declarations.
> * filter.c: New file.
> * filter.h: Change declarations.
> * filter_action.c: New file.
> * filter_expression.c: Likewise.
> * filter_qualify.c: Generate new filters.
> * Makefile.am (strace_SOURCES): Add new files.
> * strace.c (droptcb): Remove inject_vec freeing.
> (init): Change qualify to parse_qualify_filter.
>  Add sorting of filter actions.
> (trace_syscall): Add filtering after syscall decoding.
> * syscall.c (decode_socket_subcall): Remove qual_flags from decoder.
> (decode_ipc_subcall): Likewise.
> (decode_mips_subcall): Likewise.
> (get_scno): Likewise.
> (inject_vec, tcb_inject_opts, tamper_with_syscall_entering,
>  tamper_with_syscall_exiting): Remove inject_vec support code.
> (dumpio): Check tcp->qual_flg instead of global set.
> ---
>  Makefile.am         |   4 ++
>  basic_actions.c     |  98 +++++++++++++++++++++++++
>  basic_filters.c     | 107 ++++++++++++++++++++-------
>  defs.h              |  15 ++--
>  filter.c            | 147 +++++++++++++++++++++++++++++++++++++
>  filter.h            |  36 ++++++++--
>  filter_action.c     | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  filter_expression.c | 131 +++++++++++++++++++++++++++++++++
>  filter_qualify.c    | 135 +++++++++++++++++-----------------
>  strace.c            |  20 +++---
>  syscall.c           |  31 ++------
>  11 files changed, 783 insertions(+), 144 deletions(-)
>  create mode 100644 basic_actions.c
>  create mode 100644 filter.c
>  create mode 100644 filter_action.c
>  create mode 100644 filter_expression.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4f5ebfb..7299141 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -86,6 +86,7 @@ strace_SOURCES =	\
>  	affinity.c	\
>  	aio.c		\
>  	alpha.c		\
> +	basic_actions.c	\
>  	basic_filters.c	\
>  	bjm.c		\
>  	block.c		\
> @@ -125,7 +126,10 @@ strace_SOURCES =	\
>  	fetch_struct_statfs.c \
>  	file_handle.c	\
>  	file_ioctl.c	\
> +	filter_action.c	\
> +	filter_expression.c \
>  	filter_qualify.c \
> +	filter.c	\
>  	filter.h	\
>  	fs_x_ioctl.c	\
>  	flock.c		\
> diff --git a/basic_actions.c b/basic_actions.c
> new file mode 100644
> index 0000000..de87cde
> --- /dev/null
> +++ b/basic_actions.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (c) 2017 Nikolay Marchuk <marchuk.nikolay.a at gmail.com>
> + * 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"
> +
> +bool
> +is_traced(struct tcb *tcp)
> +{
> +	return (tcp->qual_flg & QUAL_TRACE);
> +}
> +
> +void *
> +parse_null(const char *str)
> +{
> +	return NULL;
> +}
> +
> +void
> +free_null(void *_priv_data)
> +{
> +	return;
> +}
> +
> +void
> +apply_trace(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_TRACE;
> +}
Missing empty line.

> +void
> +apply_inject(struct tcb *tcp, void *_priv_data)
> +{
> +	struct inject_opts **inject_vec = (struct inject_opts **)_priv_data;
> +	tcp->qual_flg |= QUAL_INJECT;
> +	tcp->inject_opts = (scno_in_range(tcp->scno) && inject_vec[current_personality])
> +	       ? &inject_vec[current_personality][tcp->scno] : NULL;
> +}
Missing empty line.

> +void*
> +parse_inject(const char *str)
> +{
> +	return NULL;
> +}
Missing empty line.

> +void free_inject(void *_priv_data)
> +{
> +	struct inject_opts **vec = (struct inject_opts **)_priv_data;
> +	unsigned int p;
> +	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p)
> +		if (vec[p])
> +			free(vec[p]);
> +	free(vec);
> +}
Missing empty line.

> +void
> +apply_read(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_READ;
> +}
Missing empty line.

> +void
> +apply_write(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_WRITE;
> +}
Missing empty line.

> +void
> +apply_raw(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_RAW;
> +}
Missing empty line.

> +void
> +apply_abbrev(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_ABBREV;
> +}
Missing empty line.

> +void
> +apply_verbose(struct tcb *tcp, void *_priv_data)
> +{
> +	tcp->qual_flg |= QUAL_VERBOSE;
> +}
> diff --git a/basic_filters.c b/basic_filters.c
> index 316b733..d0816a4 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -25,10 +25,8 @@
>   * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>   * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
> -
Why this empty line is removed?

>  #include "defs.h"
>  #include <regex.h>
> -#include "filter.h"
>  
>  typedef unsigned int number_slot_t;
>  #define BITS_PER_SLOT (sizeof(number_slot_t) * 8)
> @@ -39,6 +37,12 @@ struct number_set {
>  	bool not;
>  };
>  
> +bool
> +is_empty(struct number_set *set)
The only call site for this function is removed in the next patch.

> +{
> +	return !(set->nslots || set->not);
> +}
> +
>  static void
>  number_setbit(const unsigned int i, number_slot_t *const vec)
>  {
> @@ -48,9 +52,11 @@ number_setbit(const unsigned int i, number_slot_t *const vec)
>  static bool
>  number_isset(const unsigned int i, const number_slot_t *const vec)
>  {
> -	return vec[i / BITS_PER_SLOT] & ((number_slot_t) 1 << (i % BITS_PER_SLOT));
> +	return (vec[i / BITS_PER_SLOT]
> +	       & ((number_slot_t) 1 << (i % BITS_PER_SLOT))) ? true : false;
>  }
>  
> +
Not sure whether this additional empty line is needed here.

>  static void
>  reallocate_number_set(struct number_set *const set, const unsigned int new_nslots)
>  {
> @@ -62,7 +68,7 @@ reallocate_number_set(struct number_set *const set, const unsigned int new_nslot
>  	set->nslots = new_nslots;
>  }
>  
> -void
> +static void
>  add_number_to_set(const unsigned int number, struct number_set *const set)
>  {
>  	reallocate_number_set(set, number / BITS_PER_SLOT + 1);
> @@ -77,7 +83,7 @@ is_number_in_set(const unsigned int number, const struct number_set *const set)
>  }
>  
>  static bool
> -qualify_syscall_number(const char *s, struct number_set *set)
> +parse_syscall_number(const char *s, struct number_set *set)
>  {
>  	int n = string_to_uint(s);
>  	if (n < 0)
> @@ -108,7 +114,7 @@ regerror_msg_and_die(int errcode, const regex_t *preg,
>  }
>  
>  static bool
> -qualify_syscall_regex(const char *s, struct number_set *set)
> +parse_syscall_regex(const char *s, struct number_set *set)
>  {
>  	regex_t preg;
>  	int rc;
> @@ -180,7 +186,7 @@ lookup_class(const char *s)
>  }
>  
>  static bool
> -qualify_syscall_class(const char *s, struct number_set *set)
> +parse_syscall_class(const char *s, struct number_set *set)
>  {
>  	const unsigned int n = lookup_class(s);
>  	if (!n)
> @@ -203,7 +209,7 @@ qualify_syscall_class(const char *s, struct number_set *set)
>  }
>  
>  static bool
> -qualify_syscall_name(const char *s, struct number_set *set)
> +parse_syscall_name(const char *s, struct number_set *set)
>  {
>  	unsigned int p;
>  	bool found = false;
> @@ -224,8 +230,9 @@ qualify_syscall_name(const char *s, struct number_set *set)
>  	return found;
>  }
>  
> +
Not sure whether this additional empty line is needed here, I do not see
the system behind adding them currently.

>  static bool
> -qualify_syscall(const char *token, struct number_set *set)
> +parse_syscall(const char *token, struct number_set *set)
>  {
>  	bool ignore_fail = false;
>  
> @@ -234,21 +241,17 @@ qualify_syscall(const char *token, struct number_set *set)
>  		ignore_fail = true;
>  	}
>  	if (*token >= '0' && *token <= '9')
> -		return qualify_syscall_number(token, set) || ignore_fail;
> +		return parse_syscall_number(token, set) || ignore_fail;
>  	if (*token == '/')
> -		return qualify_syscall_regex(token + 1, set) || ignore_fail;
> -	return qualify_syscall_class(token, set)
> -	       || qualify_syscall_name(token, set)
> +		return parse_syscall_regex(token + 1, set) || ignore_fail;
> +	return parse_syscall_class(token, set)
> +	       || parse_syscall_name(token, set)
>  	       || ignore_fail;
>  }
>  
> -/*
> - * Add syscall numbers to SETs for each supported personality
> - * according to STR specification.
> - */
Removing comments and not adding new ones looks kinda ironic in the
light of the absence of any implementation description.

> -void
> -qualify_syscall_tokens(const char *const str, struct number_set *const set,
> -		       const char *const name)
> +static void
> +parse_syscall_set(const char *const str, struct number_set *const set,
> +                  const char *const name)
strace uses different indentation style for line continuations -
continuations are indented with tabs (of size 8) until this is possible
and the remainder is filled with spaces. You can use the indentation
style of the removed line as a reference.

>  {
>  	/* Clear all sets. */
>  	unsigned int p;
> @@ -298,7 +301,7 @@ handle_inversion:
>  
>  	for (token = strtok_r(copy, ",", &saveptr); token;
>  	     token = strtok_r(NULL, ",", &saveptr)) {
> -		done = qualify_syscall(token, set);
> +		done = parse_syscall(token, set);
>  		if (!done) {
>  			error_msg_and_die("invalid %s '%s'", name, token);
>  		}
> @@ -311,11 +314,37 @@ handle_inversion:
>  	}
>  }
>  
> -/*
> - * Add numbers to SET according to STR specification.
> - */
> +void*
> +parse_syscall_filter(const char *str, const char *const name)
> +{
> +	struct number_set *set = xcalloc(SUPPORTED_PERSONALITIES,
> +	                                 sizeof(struct number_set));
> +	parse_syscall_set(str, set, name);
> +	return set;
> +}
> +
> +bool
> +run_syscall_filter(struct tcb *tcp, void *_priv_data)
> +{
> +	struct number_set *set = (struct number_set *)_priv_data;
Missing space before _priv_data.

> +	return is_number_in_set(tcp->scno, &set[current_personality]);
> +}
> +
>  void
> -qualify_tokens(const char *const str, struct number_set *const set,
> +free_syscall_filter(void *_priv_data)
> +{
> +	struct number_set *set = (struct number_set *)_priv_data;
Missing space before _priv_data.

> +	unsigned int p;
> +	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> +		free(set[p].vec);
> +	}
> +	free(set);
> +}
> +
> +typedef int (*string_to_uint_func)(const char *);
This type is also defined in filters.h, does this duplication needed?

> +
> +void
> +parse_set(const char *const str, struct number_set *const set,
>  	       string_to_uint_func func, const char *const name)
>  {
>  	/* Clear the set. */
> @@ -373,3 +402,31 @@ handle_inversion:
>  		error_msg_and_die("invalid %s '%s'", name, str);
>  	}
>  }
> +
> +void*
> +parse_fd_filter(const char *str, const char *const name)
> +{
> +	struct number_set *set = xmalloc(sizeof(struct number_set));
> +	memset(set, 0, sizeof(struct number_set));
> +	parse_set(str, set, string_to_uint, name);
> +	return set;
> +}
> +
> +bool
> +run_fd_filter(struct tcb *tcp, void *_priv_data)
> +{
> +	int fd = tcp->u_arg[0];
> +	if (fd < 0)
> +		return false;
> +	struct number_set *set = (struct number_set *)_priv_data;
Missing space before _priv_data.

> +	return is_number_in_set(fd, set);
> +}
> +
> +void
> +free_fd_filter(void *_priv_data)
> +{
> +	struct number_set *set = (struct number_set *)_priv_data;
Missing space before _priv_data.

> +	free(set->vec);
> +	free(set);
> +	return;
> +}
> diff --git a/defs.h b/defs.h
> index 29a681b..9242b16 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -228,7 +228,7 @@ struct tcb {
>  	void (*_free_priv_data)(void *); /* Callback for freeing priv_data */
>  	const struct_sysent *s_ent; /* sysent[scno] or dummy struct for bad scno */
>  	const struct_sysent *s_prev_ent; /* for "resuming interrupted SYSCALL" msg */
> -	struct inject_opts *inject_vec[SUPPORTED_PERSONALITIES];
> +	struct inject_opts *inject_opts;
>  	struct timeval stime;	/* System time usage as of last process wait */
>  	struct timeval dtime;	/* Delta for system time usage */
>  	struct timeval etime;	/* Syscall entry time */
> @@ -272,8 +272,8 @@ struct tcb {
>  #define QUAL_RAW	0x008	/* print all args in hex for this syscall */
>  #define QUAL_INJECT	0x010	/* tamper with this system call on purpose */
>  #define QUAL_SIGNAL	0x100	/* report events with this signal */
> -#define QUAL_READ	0x200	/* dump data read from this file descriptor */
> -#define QUAL_WRITE	0x400	/* dump data written to this file descriptor */
> +#define QUAL_READ	0x200	/* dump data read in this syscall */
> +#define QUAL_WRITE	0x400	/* dump data written in this syscall */
>  
>  #define DEFAULT_QUAL_FLAGS (QUAL_TRACE | QUAL_ABBREV | QUAL_VERBOSE)
>  
> @@ -665,13 +665,12 @@ print_struct_statfs64(struct tcb *, kernel_ulong_t addr, kernel_ulong_t size);
>  extern void print_ifindex(unsigned int);
>  
>  struct number_set;
> -extern struct number_set read_set;
> -extern struct number_set write_set;
>  extern struct number_set signal_set;
>  
> -extern bool is_number_in_set(unsigned int number, const struct number_set *);
> -extern void qualify(const char *);
> -extern unsigned int qual_flags(const unsigned int);
> +extern bool is_number_in_set(const unsigned int, const struct number_set *);
This change of the type of the first argument looks random in the
context of the commit description (taking into account the fact that
second argument is of type const struct number_set *const in the
definition).

> +extern void sort_filter_actions(void);
> +extern void filter_syscall(struct tcb *);
> +extern void parse_qualify_filter(const char *);
>  
>  #define DECL_IOCTL(name)						\
>  extern int								\
> diff --git a/filter.c b/filter.c
> new file mode 100644
> index 0000000..1f2d2eb
> --- /dev/null
> +++ b/filter.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2017 Nikolay Marchuk <marchuk.nikolay.a at gmail.com>
> + * 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 <ctype.h>
> +#include "filter.h"
> +
> +#define DECL_FILTER(name)					\
> +extern void*							\
> +parse_ ## name ## _filter(const char *, const char *const);	\
> +extern bool							\
> +run_ ## name ## _filter(struct tcb *, void *);			\
> +extern void							\
> +free_ ## name ## _filter(void *)
Missing indentation of the macro definition.

> +DECL_FILTER(syscall);
> +DECL_FILTER(fd);
> +DECL_FILTER(path);
> +#undef DECL_FILTER
> +
> +#define FILTER_TYPE(name)					\
> +{#name, sizeof(#name) - 1, parse_ ## name ## _filter,		\
> +	run_ ## name ## _filter, free_ ## name ## _filter}
Missing indentation of the macro definition.

> +
> +static const struct filter_type {
> +	const char *name;
> +	const size_t name_len;
> +	void *(*parse_filter)(const char *, const char *const);
> +	bool (*run_filter)(struct tcb *, void *);
> +	void (*free_priv_data)(void *);
> +} filter_types[] = {
> +	FILTER_TYPE(syscall),
> +	FILTER_TYPE(fd)
> +};
> +
> +#undef FILTER_TYPE
> +
> +struct filter {
> +	const struct filter_type *type;
> +	void *_priv_data;
> +};
> +
> +static const struct filter_type *
> +lookup_filter_type(const char *str)
> +{
> +	unsigned int i;
> +	for (i = 0; i < ARRAY_SIZE(filter_types); i++) {
> +		const char *name = filter_types[i].name;
> +		const size_t len = filter_types[i].name_len;
> +		if (!strncmp(name, str, len) && !isalpha(str[len])) {
I just realised that with the len being as it is defined in FILTER_TYPE,
this call succeedes for filter types which have the same prefix as the
existing ones, which is probably not the intended behaviour.  So, len
shouldn't be decremented by one in the definition (my bad), or this call
should be replaced by strcmp, as I still see no benefit of using strncmp
over it in the absence of user-defined filter types.

> +			return &filter_types[i];
> +		}
> +	}
> +	return NULL;
> +}
> +
> +struct filter *
> +add_filter_to_array(struct filter **filters, unsigned int *nfilters,
> +                    const char *name)
> +{
> +	const struct filter_type *type = lookup_filter_type(name);
> +	if (!type)
> +		return NULL;
> +	*filters = xreallocarray(*filters, ++(*nfilters),
> +	                         sizeof(struct filter));
> +	struct filter *filter = &((*filters)[*nfilters - 1]);
> +	filter->type = type;
> +	return filter;
> +}
> +
> +void
> +parse_filter(struct filter *filter, const char *str, const char *const name)
> +{
> +	filter->_priv_data = filter->type->parse_filter(str, name);
> +}
> +
> +static bool
> +run_filter(struct tcb *tcp, struct filter *filter)
> +{
> +	return filter->type->run_filter(tcp, filter->_priv_data);
> +}
> +
> +bool *
> +run_filters(struct tcb *tcp, struct filter *filters, unsigned int nfilters)
> +{
> +	bool *variables = xcalloc(nfilters, sizeof(bool));
> +	unsigned int i;
> +	for (i = 0; i < nfilters; ++i) {
> +		variables[i] = run_filter(tcp, &filters[i]);
> +	}
> +	return variables;
> +}
> +
> +void
> +free_filter(struct filter *filter)
> +{
> +	if (!filter)
> +		return;
> +	filter->type->free_priv_data(filter->_priv_data);
> +}
> +
> +void *
> +get_filter_priv_data(struct filter *filter)
> +{
> +	return filter ? filter->_priv_data : NULL;
> +}
> +
> +void
> +set_filter_priv_data(struct filter *filter, void *_priv_data)
> +{
> +	if (filter)
> +		filter->_priv_data = _priv_data;
> +}
> +
> +void
> +set_filters_qualify_mode(struct filter **filters, unsigned int *nfilters)
> +{
> +	unsigned int i;
> +	for (i = 0; i < *nfilters - 1; ++i) {
> +		free_filter(*filters + i);
> +	}
> +	**filters = (*filters)[*nfilters - 1];
> +	*filters = xreallocarray(*filters, 1, sizeof(struct filter));
I'm pretty sure realloc() can't be expected to be able to handle re-allocating
memory hunk which is provided by pointer not pointing at the beginning
of the hunk. IOW, allocators are not expected to work with arbitrary pointers,
only with ones that are returned by those allocators.

> +	*nfilters = 1;
> +}
> diff --git a/filter.h b/filter.h
> index a798199..ed435c8 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -28,11 +28,37 @@
>  # define STRACE_FILTER_H
>  # include "defs.h"
>  
> +struct filter;
> +
> +struct filter_action;
> +
> +struct bool_expression;
> +
> +bool is_empty(const struct number_set *);
> +
> +extern bool is_traced(struct tcb *);
> +
>  typedef int (*string_to_uint_func)(const char *);
> +void parse_set(const char *const, struct number_set *const,
> +               string_to_uint_func, const char *const);
> +
> +struct filter* add_filter_to_array(struct filter **, unsigned int *nfilters,
> +                                   const char *name);
> +void parse_filter(struct filter *, const char *str, const char *const name);
> +bool *run_filters(struct tcb *, struct filter *, unsigned int);
> +void free_filter(struct filter *);
> +void *get_filter_priv_data(struct filter *);
> +void set_filter_priv_data(struct filter *, void *);
> +void set_filters_qualify_mode(struct filter **, unsigned int *nfilters);
> +
> +struct filter *create_filter(struct filter_action *, const char *name);
> +void set_qualify_mode(struct filter_action *);
> +struct filter_action *find_or_add_action(const char *);
> +void *get_filter_action_priv_data(struct filter_action *);
> +void set_filter_action_priv_data(struct filter_action *, void *);
> +
> +struct bool_expression *create_expression();
> +void set_expression_qualify_mode(struct bool_expression *);
> +bool run_expression(struct bool_expression *, unsigned int, bool *);
>  
> -void add_number_to_set(const unsigned int, struct number_set *const);
> -void qualify_tokens(const char *const, struct number_set *const,
> -                    string_to_uint_func, const char *const);
> -void qualify_syscall_tokens(const char *const, struct number_set *const,
> -                            const char *const);
>  #endif
> diff --git a/filter_action.c b/filter_action.c
> new file mode 100644
> index 0000000..f71f140
> --- /dev/null
> +++ b/filter_action.c
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright (c) 2017 Nikolay Marchuk <marchuk.nikolay.a at gmail.com>
> + * 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 <ctype.h>
> +#include "filter.h"
> +
> +#define DECL_FILTER_ACTION(name) 		\
Again, tabs after spaces.

> +extern void					\
> +apply_ ## name(struct tcb *, void *)
Missing indentation of the macro definition.

> +
> +DECL_FILTER_ACTION(trace);
> +DECL_FILTER_ACTION(inject);
> +DECL_FILTER_ACTION(read);
> +DECL_FILTER_ACTION(write);
> +DECL_FILTER_ACTION(raw);
> +DECL_FILTER_ACTION(abbrev);
> +DECL_FILTER_ACTION(verbose);
> +
> +#undef DECL_FILTER_ACTION
> +
> +#define DECL_FILTER_ACTION_PARSER(name)		\
> +extern void *					\
> +parse_ ## name(const char *);			\
> +extern void					\
> +free_ ## name(void *)
Missing indentation of the macro definition.

> +
> +DECL_FILTER_ACTION_PARSER(null);
> +DECL_FILTER_ACTION_PARSER(inject);
> +
> +#undef DECL_FILTER_ACTION_PARSER
> +
> +#define FILTER_ACTION_TYPE(NAME, PRIORITY, PARSER, PREFILTER)		\
> +{#NAME, sizeof(#NAME) - 1,PRIORITY, parse_ ## PARSER, free_ ## PARSER,	\
> + PREFILTER, apply_ ## NAME}
Missing indentation of the macro definition. Incorrect indentation of
the structure definition continuation.

> +
> +static const struct filter_action_type {
> +	const char *name;
> +	const size_t name_len;
> +	unsigned int priority;
> +	void *(*parse_args)(const char *);
> +	void (*free_priv_data)(void *);
> +	bool (*prefilter)(struct tcb *);
> +	void (*apply)(struct tcb *, void *);
> +} action_types[] = {
> +	FILTER_ACTION_TYPE(trace,	2,	null,	NULL),
> +	FILTER_ACTION_TYPE(inject,	2,	inject,	NULL),
> +	FILTER_ACTION_TYPE(read,	1,	null,	is_traced),
> +	FILTER_ACTION_TYPE(write,	1,	null,	is_traced),
> +	FILTER_ACTION_TYPE(raw,		1,	null,	is_traced),
> +	FILTER_ACTION_TYPE(abbrev,	1,	null,	is_traced),
> +	FILTER_ACTION_TYPE(verbose,	1,	null,	is_traced)
> +};
> +
> +#undef FILTER_ACTION_TYPE
> +
> +struct filter_action {
> +	const struct filter_action_type *type;
> +	struct bool_expression *expr;
> +	unsigned int nfilters;
> +	struct filter *filters;
> +	void* _priv_data;
> +};
> +
> +static struct filter_action *filter_actions;
> +static unsigned int nfilter_actions;
> +
> +static int
> +compare_action_priority(const void *a, const void *b)
> +{
> +	unsigned int priority_a = ((struct filter_action *)a)->type->priority;
Missing space before a.

> +	unsigned int priority_b = ((struct filter_action *)b)->type->priority;
Missing space before b.

> +	return (priority_a > priority_b) ? -1 :
> +	       (priority_a < priority_b) ? 1 : 0;
> +}
> +
> +void
> +sort_filter_actions(void)
> +{
> +	qsort(filter_actions, nfilter_actions, sizeof(struct filter_action),
> +	      &compare_action_priority);
> +}
> +
> +static const struct filter_action_type *
> +lookup_filter_action_type(const char *str)
> +{
> +	unsigned int i;
> +	for (i = 0; i < ARRAY_SIZE(action_types); ++i) {
> +		const char *name = action_types[i].name;
> +		const size_t len = action_types[i].name_len;
> +		if (!strncmp(name, str, len) && !isalpha(str[len])) {
> +			return &action_types[i];
> +		}
> +	}
> +	return NULL;
> +}
> +
> +static struct filter_action *
> +add_action(const char *name)
> +{
> +	const struct filter_action_type *type = lookup_filter_action_type(name);
> +	if (!type)
> +		error_msg_and_die("invalid filter action '%s'", name);
> +	filter_actions = xreallocarray(filter_actions, ++nfilter_actions,
> +	                               sizeof(struct filter_action));
> +	struct filter_action *action = &filter_actions[nfilter_actions - 1];
> +	memset(action, 0, sizeof(*action));
> +	action->type = type;
> +	action->expr = create_expression();
> +	return action;
> +}
> +
> +struct filter_action *
> +find_or_add_action(const char *name)
> +{
> +	const struct filter_action_type *type = lookup_filter_action_type(name);
> +	if (!type)
> +		error_msg_and_die("invalid filter action '%s'", name);
> +	if (type->parse_args != &parse_null)
> +		return add_action(name);
> +	unsigned int i;
> +	for (i = 0; i < nfilter_actions; i++) {
> +		if (filter_actions[i].type == type)
> +			return &filter_actions[i];
> +	}
> +	return add_action(name);
> +}
> +
> +static void
> +run_filter_action(struct tcb *tcp, struct filter_action *action)
> +{
> +	if (action->type->prefilter && !action->type->prefilter(tcp))
> +		return;
> +	bool *variables = run_filters(tcp, action->filters, action->nfilters);
> +	bool res = run_expression(action->expr, action->nfilters, variables);
> +	free(variables);
I'm not a big fan of allocating and freeing variables on each processed
call, we probably can allocate them only once since nfilters doesn't
changes during the strace's run and they are initialised in full on each
run_filters() call.

> +	if (res) {
> +		action->type->apply(tcp, action->_priv_data);
> +	}
> +}
> +
> +struct filter *
> +create_filter(struct filter_action *action, const char *name)
> +{
> +	if (!action)
> +		error_msg_and_die("invalid filter action");
> +	return add_filter_to_array(&action->filters,
> +	                           &action->nfilters, name);
> +}
> +
> +void
> +set_qualify_mode(struct filter_action *action)
> +{
> +	if ((!action) || action->nfilters == 0)
> +		error_msg_and_die("invalid filter action");
> +	set_filters_qualify_mode(&action->filters, &action->nfilters);
> +	set_expression_qualify_mode(action->expr);
> +}
> +
> +void
> +filter_syscall(struct tcb *tcp)
> +{
> +	unsigned int i;
> +	for (i = 0; i < nfilter_actions; i++) {
> +		run_filter_action(tcp, &filter_actions[i]);
> +	}
> +}
> +
> +void *
> +get_filter_action_priv_data(struct filter_action *action)
> +{
> +	return action ? action->_priv_data : NULL;
> +}
> +
> +void
> +set_filter_action_priv_data(struct filter_action *action, void *_priv_data)
> +{
> +	if (action)
> +		action->_priv_data = _priv_data;
> +}
> diff --git a/filter_expression.c b/filter_expression.c
> new file mode 100644
> index 0000000..ca02280
> --- /dev/null
> +++ b/filter_expression.c
> @@ -0,0 +1,131 @@
> +/*
> + * Copyright (c) 2017 Nikolay Marchuk <marchuk.nikolay.a at gmail.com>
> + * 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"
> +
> +struct expression_token{
Missing space before the curly bracket.

> +	enum token_type{
Missing space before the curly bracket.

> +		TOK_VARIABLE,
> +		TOK_OPERATOR
> +	} type;
> +	union token_data{
Missing space before the curly bracket.

> +		unsigned int variable_id;
> +		enum operator_type{
Missing space before the curly bracket.

> +			OP_NOT,
> +			OP_AND,
> +			OP_OR
> +		} operator_id;
> +	} data;
> +};
> +
> +struct bool_expression{
Missing space before the curly bracket.

> +	unsigned int ntokens;
> +	struct expression_token *tokens;
> +};
> +
> +struct bool_expression *
> +create_expression(void)
> +{
> +	struct bool_expression *expr = xmalloc(sizeof(struct bool_expression));
> +	memset(expr, 0, sizeof(struct bool_expression));
> +	return expr;
> +}
> +
> +static void
> +reallocate_expression(struct bool_expression *const expr, const unsigned int new_ntokens)
Oversized string.

> +{
> +	if (new_ntokens <= expr->ntokens)
> +		return;
> +	expr->tokens = xreallocarray(expr->tokens, new_ntokens,
> +	                             sizeof(*expr->tokens));
> +	memset(expr->tokens + expr->ntokens, 0,
> +	       sizeof(*expr->tokens) * (new_ntokens - expr->ntokens));
> +	expr->ntokens = new_ntokens;
> +}
> +
> +#define STACK_SIZE 32
> +static bool stack[STACK_SIZE];
> +
> +void
> +set_expression_qualify_mode(struct bool_expression *expr)
> +{
> +	if (!expr)
> +		error_msg_and_die("invalid expression");
> +	reallocate_expression(expr, 1);
> +	expr->tokens[0].type = TOK_VARIABLE;
> +	expr->tokens[0].data.variable_id = 0;
> +}
> +
> +bool
> +run_expression(struct bool_expression *expr,
> +               unsigned int variables_num, bool *variables)
> +{
> +	/* Current stack index */
> +	unsigned int index = 0;
> +	struct expression_token *tok = expr->tokens;
> +	struct expression_token *const tok_end = expr->tokens + expr->ntokens;
> +	for (; tok != tok_end; ++tok) {
> +		switch (tok->type) {
> +		case TOK_VARIABLE:
> +			if (index == STACK_SIZE)
> +				error_msg_and_die("stack overflow");
> +			if (tok->data.variable_id >= variables_num)
> +				error_msg_and_die("corrupted filter "
> +				                  "expression");
> +			stack[index] = variables[tok->data.variable_id];
> +			index++;
> +			break;
> +		case TOK_OPERATOR:
> +			switch (tok->data.operator_id) {
> +			case OP_NOT:
> +				if (index == 0)
> +					error_msg_and_die("corrupted filter "
> +					                  "expression");
> +				stack[index - 1] = !stack[index - 1];
> +				break;
> +			case OP_AND:
> +				if (index < 2)
> +					error_msg_and_die("corrupted filter "
> +					                  "expression");
> +				stack[index - 2] = stack[index - 2]
> +				                   && stack[index - 1];
> +				--index;
> +				break;
> +			case OP_OR:
> +				if (index < 2)
> +					error_msg_and_die("corrupted filter "
> +					                  "expression");
> +				stack[index - 2] = stack[index - 2]
> +				                   || stack[index - 1];
> +				--index;
> +				break;
> +			}
> +		}
> +	}
> +	if (index != 1)
> +		error_msg_and_die("corrupted filter expression");
> +	return stack[0];
> +}
> diff --git a/filter_qualify.c b/filter_qualify.c
> index e23a9f1..6020ce8 100644
> --- a/filter_qualify.c
> +++ b/filter_qualify.c
> @@ -25,13 +25,11 @@
>   * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>   * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>   */
> -
Why this empty line is removed?

>  #include "defs.h"
>  #include "nsig.h"
>  #include "filter.h"
>  
>  typedef unsigned int number_slot_t;
> -#define BITS_PER_SLOT (sizeof(number_slot_t) * 8)
Why it was here in the first place?

>  
>  struct number_set {
>  	number_slot_t *vec;
> @@ -39,16 +37,8 @@ struct number_set {
>  	bool not;
>  };
>  
> -struct number_set read_set;
> -struct number_set write_set;
>  struct number_set signal_set;
>  
> -static struct number_set abbrev_set[SUPPORTED_PERSONALITIES];
> -static struct number_set inject_set[SUPPORTED_PERSONALITIES];
> -static struct number_set raw_set[SUPPORTED_PERSONALITIES];
> -static struct number_set trace_set[SUPPORTED_PERSONALITIES];
> -static struct number_set verbose_set[SUPPORTED_PERSONALITIES];
> -
>  static int
>  find_errno_by_name(const char *name)
>  {
> @@ -184,49 +174,67 @@ parse_error:
>  }
>  
>  static void
> -qualify_read(const char *const str)
> +parse_read(const char *const str)
>  {
> -	qualify_tokens(str, &read_set, string_to_uint, "descriptor");
> +	struct filter_action *action = find_or_add_action("read");
> +	struct filter *filter = create_filter(action, "fd");
> +	parse_filter(filter, str, "descriptor");
> +	set_qualify_mode(action);
>  }
>  
>  static void
> -qualify_write(const char *const str)
> +parse_write(const char *const str)
>  {
> -	qualify_tokens(str, &write_set, string_to_uint, "descriptor");
> +	struct filter_action *action = find_or_add_action("write");
> +	struct filter *filter = create_filter(action, "fd");
> +	parse_filter(filter, str, "descriptor");
> +	set_qualify_mode(action);
>  }
>  
>  static void
>  qualify_signals(const char *const str)
>  {
> -	qualify_tokens(str, &signal_set, sigstr_to_uint, "signal");
> +	parse_set(str, &signal_set, sigstr_to_uint, "signal");
>  }
>  
>  static void
> -qualify_trace(const char *const str)
> +parse_trace(const char *const str)
>  {
> -	qualify_syscall_tokens(str, trace_set, "system call");
> +	struct filter_action *action = find_or_add_action("trace");
> +	struct filter *filter = create_filter(action, "syscall");
> +	parse_filter(filter, str, "system call");
> +	set_qualify_mode(action);
>  }
>  
>  static void
> -qualify_abbrev(const char *const str)
> +parse_abbrev(const char *const str)
>  {
> -	qualify_syscall_tokens(str, abbrev_set, "system call");
> +	struct filter_action *action = find_or_add_action("abbrev");
> +	struct filter *filter = create_filter(action, "syscall");
> +	parse_filter(filter, str, "system call");
> +	set_qualify_mode(action);
>  }
>  
>  static void
> -qualify_verbose(const char *const str)
> +parse_verbose(const char *const str)
>  {
> -	qualify_syscall_tokens(str, verbose_set, "system call");
> +	struct filter_action *action = find_or_add_action("verbose");
> +	struct filter *filter = create_filter(action, "syscall");
> +	parse_filter(filter, str, "system call");
> +	set_qualify_mode(action);
>  }
>  
>  static void
> -qualify_raw(const char *const str)
> +parse_raw(const char *const str)
>  {
> -	qualify_syscall_tokens(str, raw_set, "system call");
> +	struct filter_action *action = find_or_add_action("raw");
> +	struct filter *filter = create_filter(action, "syscall");
> +	parse_filter(filter, str, "system call");
> +	set_qualify_mode(action);
>  }
>  
>  static void
> -qualify_inject_common(const char *const str,
> +parse_inject_common(const char *const str,
>  		      const bool fault_tokens_only,
>  		      const char *const description)
>  {
> @@ -253,19 +261,23 @@ qualify_inject_common(const char *const str,
>  		}
>  	}
>  
> -	struct number_set tmp_set[SUPPORTED_PERSONALITIES];
> -	memset(tmp_set, 0, sizeof(tmp_set));
> -	qualify_syscall_tokens(name, tmp_set, description);
> +	struct filter_action *action = find_or_add_action("inject");
> +	struct filter *filter = create_filter(action, "syscall");
> +	parse_filter(filter, name, description);
> +	set_qualify_mode(action);
>  
>  	free(buf);
> -
> +	const struct number_set *tmp_set = (const struct number_set *)
> +	                                    get_filter_priv_data(filter);
> +	struct inject_opts **inject_vec = xcalloc(SUPPORTED_PERSONALITIES,
> +	                                          sizeof(struct inject_opts *));
>  	/*
>  	 * Initialize inject_vec accourding to tmp_set.
"according"

> -	 * Merge tmp_set into inject_set.
>  	 */
After the removal of the second line the comment can be edited in the
single line.

> +
>  	unsigned int p;
>  	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> -		if (!tmp_set[p].nslots && !tmp_set[p].not) {
> +		if (is_empty(&tmp_set[p])) {
>  			continue;
>  		}
>  
> @@ -277,54 +289,53 @@ qualify_inject_common(const char *const str,
>  		unsigned int i;
>  		for (i = 0; i < nsyscall_vec[p]; ++i) {
>  			if (is_number_in_set(i, &tmp_set[p])) {
> -				add_number_to_set(i, &inject_set[p]);
>  				inject_vec[p][i] = opts;
>  			}
>  		}
> -
> -		free(tmp_set[p].vec);
>  	}
> +	set_filter_action_priv_data(action, inject_vec);
>  }
>  
>  static void
> -qualify_fault(const char *const str)
> +parse_fault(const char *const str)
>  {
> -	qualify_inject_common(str, true, "fault argument");
> +	parse_inject_common(str, true, "fault argument");
>  }
>  
>  static void
> -qualify_inject(const char *const str)
> +parse_inject(const char *const str)
>  {
> -	qualify_inject_common(str, false, "inject argument");
> +	parse_inject_common(str, false, "inject argument");
>  }
>  
>  static const struct qual_options {
>  	const char *name;
>  	void (*qualify)(const char *);
>  } qual_options[] = {
> -	{ "trace",	qualify_trace	},
> -	{ "t",		qualify_trace	},
> -	{ "abbrev",	qualify_abbrev	},
> -	{ "a",		qualify_abbrev	},
> -	{ "verbose",	qualify_verbose	},
> -	{ "v",		qualify_verbose	},
> -	{ "raw",	qualify_raw	},
> -	{ "x",		qualify_raw	},
> +	{ "trace",	parse_trace	},
> +	{ "t",		parse_trace	},
> +	{ "abbrev",	parse_abbrev	},
> +	{ "a",		parse_abbrev	},
> +	{ "verbose",	parse_verbose	},
> +	{ "v",		parse_verbose	},
> +	{ "raw",	parse_raw	},
> +	{ "x",		parse_raw	},
>  	{ "signal",	qualify_signals	},
>  	{ "signals",	qualify_signals	},
>  	{ "s",		qualify_signals	},
> -	{ "read",	qualify_read	},
> -	{ "reads",	qualify_read	},
> -	{ "r",		qualify_read	},
> -	{ "write",	qualify_write	},
> -	{ "writes",	qualify_write	},
> -	{ "w",		qualify_write	},
> -	{ "fault",	qualify_fault	},
> -	{ "inject",	qualify_inject	},
> +	{ "read",	parse_read	},
> +	{ "reads",	parse_read	},
> +	{ "r",		parse_read	},
> +	{ "write",	parse_write	},
> +	{ "writes",	parse_write	},
> +	{ "w",		parse_write	},
> +	{ "fault",	parse_fault	},
> +	{ "inject",	parse_inject	},
>  };
>  
> +
>  void
> -qualify(const char *str)
> +parse_qualify_filter(const char *str)
>  {
>  	const struct qual_options *opt = qual_options;
>  	unsigned int i;
> @@ -340,21 +351,5 @@ qualify(const char *str)
>  		opt = &qual_options[i];
>  		break;
>  	}
> -
Why this line is removed?

>  	opt->qualify(str);
>  }
> -
> -unsigned int
> -qual_flags(const unsigned int scno)
> -{
> -	return	(is_number_in_set(scno, &trace_set[current_personality])
> -		   ? QUAL_TRACE : 0)
> -		| (is_number_in_set(scno, &abbrev_set[current_personality])
> -		   ? QUAL_ABBREV : 0)
> -		| (is_number_in_set(scno, &verbose_set[current_personality])
> -		   ? QUAL_VERBOSE : 0)
> -		| (is_number_in_set(scno, &raw_set[current_personality])
> -		   ? QUAL_RAW : 0)
> -		| (is_number_in_set(scno, &inject_set[current_personality])
> -		   ? QUAL_INJECT : 0);
> -}
> diff --git a/strace.c b/strace.c
> index 614ab9e..e48ed76 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -822,10 +822,6 @@ droptcb(struct tcb *tcp)
>  	if (tcp->pid == 0)
>  		return;
>  
> -	int p;
> -	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p)
> -		free(tcp->inject_vec[p]);
> -
>  	free_tcb_priv_data(tcp);
>  
>  #ifdef USE_LIBUNWIND
> @@ -1626,13 +1622,13 @@ init(int argc, char *argv[])
>  	shared_log = stderr;
>  	set_sortby(DEFAULT_SORTBY);
>  	set_personality(DEFAULT_PERSONALITY);
> -	qualify("trace=all");
> -	qualify("abbrev=all");
> -	qualify("verbose=all");
> +	parse_qualify_filter("trace=all");
> +	parse_qualify_filter("abbrev=all");
> +	parse_qualify_filter("verbose=all");
>  #if DEFAULT_QUAL_FLAGS != (QUAL_TRACE | QUAL_ABBREV | QUAL_VERBOSE)
>  # error Bug in DEFAULT_QUAL_FLAGS
>  #endif
> -	qualify("signal=all");
> +	parse_qualify_filter("signal=all");
>  	while ((c = getopt(argc, argv,
>  		"+b:cCdfFhiqrtTvVwxyz"
>  #ifdef USE_LIBUNWIND
> @@ -1699,7 +1695,7 @@ init(int argc, char *argv[])
>  			show_fd_path++;
>  			break;
>  		case 'v':
> -			qualify("abbrev=none");
> +			parse_qualify_filter("abbrev=none");
>  			break;
>  		case 'V':
>  			print_version();
> @@ -1714,7 +1710,7 @@ init(int argc, char *argv[])
>  				error_opt_arg(c, optarg);
>  			break;
>  		case 'e':
> -			qualify(optarg);
> +			parse_qualify_filter(optarg);
>  			break;
>  		case 'o':
>  			outfname = xstrdup(optarg);
> @@ -1762,6 +1758,8 @@ init(int argc, char *argv[])
>  			break;
>  		}
>  	}
> +	sort_filter_actions();
> +
>  	argv += optind;
>  	/* argc -= optind; - no need, argc is not used below */
>  
> @@ -2459,6 +2457,8 @@ trace_syscall(struct tcb *tcp, unsigned int *sig)
>  		case 0:
>  			return 0;
>  		case 1:
> +			if (!tcp->qual_flg)
> +				filter_syscall(tcp);
>  			res = syscall_entering_trace(tcp, sig);
>  		}
>  		syscall_entering_finish(tcp, res);
> diff --git a/syscall.c b/syscall.c
> index 02626c7..b6e1ba3 100644
> --- a/syscall.c
> +++ b/syscall.c
> @@ -382,7 +382,6 @@ decode_socket_subcall(struct tcb *tcp)
>  		return;
>  
>  	tcp->scno = scno;
> -	tcp->qual_flg = qual_flags(scno);
>  	tcp->s_ent = &sysent[scno];
>  
>  	unsigned int i;
> @@ -422,7 +421,6 @@ decode_ipc_subcall(struct tcb *tcp)
>  	}
>  
>  	tcp->scno = SYS_ipc_subcall + call;
> -	tcp->qual_flg = qual_flags(tcp->scno);
>  	tcp->s_ent = &sysent[tcp->scno];
>  
>  	const unsigned int n = tcp->s_ent->nargs;
> @@ -439,7 +437,6 @@ decode_mips_subcall(struct tcb *tcp)
>  	if (!scno_is_valid(tcp->u_arg[0]))
>  		return;
>  	tcp->scno = tcp->u_arg[0];
> -	tcp->qual_flg = qual_flags(tcp->scno);
>  	tcp->s_ent = &sysent[tcp->scno];
>  	memmove(&tcp->u_arg[0], &tcp->u_arg[1],
>  		sizeof(tcp->u_arg) - sizeof(tcp->u_arg[0]));
> @@ -468,7 +465,7 @@ dumpio(struct tcb *tcp)
>  	if (fd < 0)
>  		return;
>  
> -	if (is_number_in_set(fd, &read_set)) {
> +	if (tcp->qual_flg & QUAL_READ) {
"Some macro can be added for this check, similar to verbose(tcp) and
alike."

>  		switch (tcp->s_ent->sen) {
>  		case SEN_read:
>  		case SEN_pread:
> @@ -491,7 +488,7 @@ dumpio(struct tcb *tcp)
>  			return;
>  		}
>  	}
> -	if (is_number_in_set(fd, &write_set)) {
> +	if (tcp->qual_flg & QUAL_WRITE) {
>  		switch (tcp->s_ent->sen) {
>  		case SEN_write:
>  		case SEN_pwrite:
> @@ -577,28 +574,10 @@ static void get_error(struct tcb *, const bool);
>  static int arch_set_error(struct tcb *);
>  static int arch_set_success(struct tcb *);
>  
> -struct inject_opts *inject_vec[SUPPORTED_PERSONALITIES];
> -
> -static struct inject_opts *
> -tcb_inject_opts(struct tcb *tcp)
> -{
> -	return (scno_in_range(tcp->scno) && tcp->inject_vec[current_personality])
> -	       ? &tcp->inject_vec[current_personality][tcp->scno] : NULL;
> -}
> -
> -
>  static long
>  tamper_with_syscall_entering(struct tcb *tcp, unsigned int *signo)
>  {
> -	if (!tcp->inject_vec[current_personality]) {
> -		tcp->inject_vec[current_personality] =
> -			xcalloc(nsyscalls, sizeof(**inject_vec));
> -		memcpy(tcp->inject_vec[current_personality],
> -		       inject_vec[current_personality],
> -		       nsyscalls * sizeof(**inject_vec));
> -	}
Note that this copy-on-demand allows tracking injection counters
per-tcb, moving this code to apply_inject breaks this behaviour.

> -
> -	struct inject_opts *opts = tcb_inject_opts(tcp);
> +	struct inject_opts *opts = tcp->inject_opts;
>  
>  	if (!opts || opts->first == 0)
>  		return 0;
> @@ -621,7 +600,7 @@ tamper_with_syscall_entering(struct tcb *tcp, unsigned int *signo)
>  static long
>  tamper_with_syscall_exiting(struct tcb *tcp)
>  {
> -	struct inject_opts *opts = tcb_inject_opts(tcp);
> +	struct inject_opts *opts = tcp->inject_opts;
>  
>  	if (!opts)
>  		return 0;
> @@ -1258,7 +1237,7 @@ get_scno(struct tcb *tcp)
>  
>  	if (scno_is_valid(tcp->scno)) {
>  		tcp->s_ent = &sysent[tcp->scno];
> -		tcp->qual_flg = qual_flags(tcp->scno);
> +		tcp->qual_flg = 0;
Why you zero qual_flg here (and not call filter_syscall())?

>  	} else {
>  		struct sysent_buf *s = xcalloc(1, sizeof(*s));
>  
> -- 
> 2.1.4
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Strace-devel mailing list
> Strace-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel




More information about the Strace-devel mailing list