[PATCH v7 2/8] Introduce new filtering architecture
Eugene Syromiatnikov
esyr at redhat.com
Mon Aug 14 00:57:31 UTC 2017
On Fri, Aug 11, 2017 at 05:43:47PM +0700, Nikolay Marchuk wrote:
> This change introduces new filtering architecture primitives: filter,
> filter_action and bool_expression. Filtering is now done after decoding of
> syscall and tcp->qual_flg stores filtering results.
>
> * basic_actions.c: New file.
> * basic_filters.c (number_isset): Return bool.
> (add_number_to_set): Make static.
> (qualify_syscall_number): Rename to parse_syscall_number.
> (qualify_syscall_regex): Rename to parse_syscall_regex.
> (qualify_syscall_class): Rename to parse_syscall_class.
> (qualify_syscall_name): Rename to parse_syscall_name.
> (qualify_syscall): Rename to parse_syscall, use renamed functions.
> (qualify_syscall_tokens): Rename to parse_syscall_set, make static,
> remove "name" argument (assume "system call"), remove 'handle_inversion'
> label, remove additional clearing of the sets.
> (qualify_tokens): Rename to parse_set, remove 'handle_inversion' label,
> remove additional clearing of the set.
> (parse_syscall_filter, parse_fd_filter, run_syscall_filter,
> run_fd_filter, free_syscall_filter, free_fd_filter): New functions.
> * defs.h (struct inject_opts): Add init flag.
> (QUAL_READ, QUAL_WRITE): Change description.
> (dump_read, dump_write): Add macros for checking QUAL_READ/QUAL_WRITE.
> (read_set, write_set): Remove global set variables.
> (qualify, qual_flags): Remove old declarations ...
> (filter_syscall, parse_qualify_filter, filtering_parsing_finish):
> ... and add new declarations.
> * filter.c: New file.
> * filter.h: Change declarations.
> * filter_action.c: New file.
> * filter_expression.c: Likewise.
> * filter_qualify.c (read_set, write_set, abbrev_set, inject_set, raw_set,
> trace_set, verbose_set): Remove set variables.
> (parse_inject_expression): Remove function.
> (parse_inject_common_args): New function.
> (qualify_read): Rename to parse_read.
> (qualify_write): Rename to parse_write.
> (qualify_signals): Use parse_set function. Add clearing of the set.
> (qualify_trace): Rename to parse_trace.
> (qualify_abbrev): Rename to parse_abbrev.
> (qualify_verbose): Rename to parse_verbose.
> (qualify_raw): Rename to parse_raw.
> (qualify_inject_common): Rename to parse_inject_common_qualify, use
> new filters.
> (qualify_fault): Rename to parse_fault.
> (qualify_inject): Rename to parse_inject.
> (qualify): Rename to parse_qualify_filter.
> (qual_flags): Remove function.
> * Makefile.am (strace_SOURCES): Add new files.
> * strace.c (init): Use new filtering for -e option.
> (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, tamper_with_syscall_entering): Remove inject_vec support code.
> (dumpio): Check new macros instead of global sets.
> * tests/filtering_syscall-syntax.test: Add syscall set syntax testing
> of inject and fault.
> * tests/qual_fault-syntax.test: Move syscall set syntax testing to
> filtering_syscall-syntax.test.
> * tests/qual_inject-syntax.test: Likewise.
I do not see reasons for having this change in this patch and not in the
previous one.
> ---
> Makefile.am | 4 +
> basic_actions.c | 147 ++++++++++++++++++++++
> basic_filters.c | 125 +++++++++++++------
> defs.h | 14 ++-
> filter.c | 146 ++++++++++++++++++++++
> filter.h | 37 +++++-
> filter_action.c | 230 ++++++++++++++++++++++++++++++++++
> filter_expression.c | 235 +++++++++++++++++++++++++++++++++++
> filter_qualify.c | 239 ++++++++++++++++--------------------
> strace.c | 18 +--
> syscall.c | 24 +---
> tests/filtering_syscall-syntax.test | 24 +++-
> tests/qual_fault-syntax.test | 26 ++--
> tests/qual_inject-syntax.test | 27 ++--
> 14 files changed, 1052 insertions(+), 244 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 5ec0fa35..84df39a8 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 \
> bind.c \
> bjm.c \
> @@ -133,7 +134,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 \
> flock.c \
> flock.h \
> diff --git a/basic_actions.c b/basic_actions.c
> new file mode 100644
> index 00000000..5a1ebf8d
> --- /dev/null
> +++ b/basic_actions.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 "filter.h"
> +
> +bool
> +is_traced(struct tcb *tcp)
> +{
> + return (tcp->qual_flg & QUAL_TRACE);
> +}
> +
> +bool
> +not_injected(struct tcb *tcp)
> +{
> + return !(tcp->qual_flg & QUAL_INJECT);
> +}
> +
> +void *
> +parse_null(const char *str)
> +{
> + return NULL;
> +}
> +
> +void
> +free_null(void *_priv_data)
> +{
> + return;
> +}
It's not used anywhere, by the way. Or, more precisely, it's not called.
> +
> +void
> +apply_trace(struct tcb *tcp, void *_priv_data)
> +{
> + if (!tracing_paths || pathtrace_match(tcp))
> + tcp->qual_flg |= QUAL_TRACE;
> +}
> +
> +void
> +apply_inject(struct tcb *tcp, void *_priv_data)
> +{
> + struct inject_opts *opts = _priv_data;
> +
> + tcp->qual_flg |= QUAL_INJECT;
> + if (!tcp->inject_vec[current_personality])
> + tcp->inject_vec[current_personality] =
> + xcalloc(nsyscalls, sizeof(struct inject_opts));
> + if (scno_in_range(tcp->scno)
> + && !tcp->inject_vec[current_personality][tcp->scno].init)
> + tcp->inject_vec[current_personality][tcp->scno] = *opts;
> +}
> +
> +static void *
> +parse_inject_common(const char *str, bool fault_tokens_only,
> + const char *description)
> +{
> + struct inject_opts *opts = xmalloc(sizeof(struct inject_opts));
> + char *buf = str ? xstrdup(str) : NULL;
Since xstrdup dies if memory is insufficient, and the idiom
"p ? xstrdup(p) : NULL" is already used more than once, I think you can
safely move this check inside xstrdup (at least I do not see this as a
drastic change in function's semantics comparing to libc counterpart).
> +
> + parse_inject_common_args(buf, opts, ";", fault_tokens_only);
> + if (!opts->init)
> + error_msg_and_die("invalid %s '%s'", description, str);
> + free(buf);
> + return opts;
> +}
> +
> +void *
> +parse_inject(const char *str)
> +{
> + return parse_inject_common(str, false, "inject argument");
I think you can safely move " argument" part of a string inside
parse_inject_common.
> +}
> +
> +void free_inject(void *_priv_data)
Function return type should on a separate line.
> +{
> + free(_priv_data);
> +}
> +
> +void
> +apply_fault(struct tcb *tcp, void *_priv_data)
> +{
> + apply_inject(tcp, _priv_data);
> +}
> +
> +void *
> +parse_fault(const char *str)
> +{
> + return parse_inject_common(str, true, "fault argument");
> +}
> +
> +void
> +free_fault(void *_priv_data)
> +{
> + free_inject(_priv_data);
> +}
> +
> +void
> +apply_read(struct tcb *tcp, void *_priv_data)
> +{
> + tcp->qual_flg |= QUAL_READ;
> +}
> +
> +void
> +apply_write(struct tcb *tcp, void *_priv_data)
> +{
> + tcp->qual_flg |= QUAL_WRITE;
> +}
> +
> +void
> +apply_raw(struct tcb *tcp, void *_priv_data)
> +{
> + tcp->qual_flg |= QUAL_RAW;
> +}
> +
> +void
> +apply_abbrev(struct tcb *tcp, void *_priv_data)
> +{
> + tcp->qual_flg |= QUAL_ABBREV;
> +}
> +
> +void
> +apply_verbose(struct tcb *tcp, void *_priv_data)
> +{
> + tcp->qual_flg |= QUAL_VERBOSE;
> +}
With the introduction of qual_flg field in struct filter_action_type,
these functions look almost redundant.
> diff --git a/basic_filters.c b/basic_filters.c
> index 1dbdeac8..6e2ec39a 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -48,7 +48,8 @@ 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;
Btw, !!() could be enough here.
> }
>
> static void
> @@ -62,7 +63,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 +78,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 +109,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 +181,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 +204,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;
> @@ -225,7 +226,7 @@ qualify_syscall_name(const char *s, struct number_set *set)
> }
>
> 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,11 +235,11 @@ 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;
> }
>
> @@ -247,24 +248,15 @@ qualify_syscall(const char *token, struct number_set *set)
> * according to STR specification.
> */
> void
> -qualify_syscall_tokens(const char *const str, struct number_set *const set,
> - const char *const name)
> +parse_syscall_set(const char *const str, struct number_set *const set)
> {
> - /* Clear all sets. */
> unsigned int p;
> - for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> - if (set[p].nslots)
> - memset(set[p].vec, 0,
> - sizeof(*set[p].vec) * set[p].nslots);
> - set[p].not = false;
> - }
> + const char *s = str;
>
> /*
> * Each leading ! character means inversion
> * of the remaining specification.
> */
> - const char *s = str;
> -handle_inversion:
> while (*s == '!') {
> for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> set[p].not = !set[p].not;
> @@ -280,16 +272,18 @@ handle_inversion:
> */
> return;
> } else if (strcmp(s, "all") == 0) {
> - s = "!none";
> - goto handle_inversion;
> + for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> + set[p].not = !set[p].not;
> + }
Curly braces can be omitted here.
> + return;
> }
>
> /*
> * Split the string into comma separated tokens.
> - * For each token, call qualify_syscall that will take care
> + * For each token, call parse_syscall that will take care
> * if adding appropriate syscall numbers to sets.
> * The absence of tokens or a negative return code
> - * from qualify_syscall is a fatal error.
> + * from parse_syscall is a fatal error.
> */
> char *copy = xstrdup(s);
> char *saveptr = NULL;
> @@ -298,37 +292,62 @@ 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);
> + error_msg_and_die("invalid system call '%s'", token);
> }
> }
>
> free(copy);
>
> if (!done) {
> - error_msg_and_die("invalid %s '%s'", name, str);
> + error_msg_and_die("invalid system call '%s'", str);
> + }
> +}
> +
> +void *
> +parse_syscall_filter(const char *str)
> +{
> + struct number_set *set = xcalloc(SUPPORTED_PERSONALITIES,
> + sizeof(struct number_set));
> +
> + parse_syscall_set(str, set);
> + return set;
> +}
> +
> +bool
> +run_syscall_filter(struct tcb *tcp, void *_priv_data)
> +{
> + struct number_set *set = _priv_data;
> +
> + return is_number_in_set(tcp->scno, &set[current_personality]);
> +}
> +
> +void
> +free_syscall_filter(void *_priv_data)
> +{
> + struct number_set *set = _priv_data;
> + unsigned int p;
> +
> + for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> + free(set[p].vec);
> }
> + free(set);
> }
>
> /*
> * Add numbers to SET according to STR specification.
> */
> void
> -qualify_tokens(const char *const str, struct number_set *const set,
> +parse_set(const char *const str, struct number_set *const set,
> string_to_uint_func func, const char *const name)
> {
> - /* Clear the set. */
> - if (set->nslots)
> - memset(set->vec, 0, sizeof(*set->vec) * set->nslots);
> - set->not = false;
> + const char *s = str;
>
> /*
> * Each leading ! character means inversion
> * of the remaining specification.
> */
> - const char *s = str;
> -handle_inversion:
> while (*s == '!') {
> set->not = !set->not;
> ++s;
> @@ -341,8 +360,8 @@ handle_inversion:
> */
> return;
> } else if (strcmp(s, "all") == 0) {
> - s = "!none";
> - goto handle_inversion;
> + set->not = !set->not;
> + return;
> }
>
> /*
> @@ -373,3 +392,33 @@ handle_inversion:
> error_msg_and_die("invalid %s '%s'", name, str);
> }
> }
> +
> +void *
> +parse_fd_filter(const char *str)
> +{
> + struct number_set *set = xcalloc(1, sizeof(struct number_set));
> +
> + parse_set(str, set, string_to_uint, "descriptor");
> + return set;
> +}
> +
> +bool
> +run_fd_filter(struct tcb *tcp, void *_priv_data)
> +{
> + int fd = tcp->u_arg[0];
> + struct number_set *set = _priv_data;
> +
> + if (fd < 0)
> + return false;
> + return is_number_in_set(fd, set);
> +}
> +
> +void
> +free_fd_filter(void *_priv_data)
> +{
> + struct number_set *set = _priv_data;
> +
> + free(set->vec);
> + free(set);
> + return;
> +}
> diff --git a/defs.h b/defs.h
> index 2efbfa36..69cd765e 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -199,6 +199,7 @@ struct inject_opts {
> uint16_t step;
> uint16_t signo;
> int rval;
> + bool init;
> };
>
> #define MAX_ERRNO_VALUE 4095
> @@ -268,8 +269,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)
>
> @@ -278,6 +279,8 @@ struct tcb {
> #define syserror(tcp) ((tcp)->u_error != 0)
> #define verbose(tcp) ((tcp)->qual_flg & QUAL_VERBOSE)
> #define abbrev(tcp) ((tcp)->qual_flg & QUAL_ABBREV)
> +#define dump_read(tcp) ((tcp)->qual_flg & QUAL_READ)
> +#define dump_write(tcp) ((tcp)->qual_flg & QUAL_WRITE)
> #define filtered(tcp) ((tcp)->flags & TCB_FILTERED)
> #define hide_log(tcp) ((tcp)->flags & TCB_HIDE_LOG)
>
> @@ -662,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 void filtering_parsing_finish(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 00000000..d51b3183
> --- /dev/null
> +++ b/filter.c
> @@ -0,0 +1,146 @@
> +/*
> + * 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 "filter.h"
> +
> +#define DECL_FILTER(name) \
> +extern void * \
> +parse_ ## name ## _filter(const char *); \
> +extern bool \
> +run_ ## name ## _filter(struct tcb *, void *); \
> +extern void \
> +free_ ## name ## _filter(void *) \
> +/* End of DECL_FILTER definition. */
> +
> +DECL_FILTER(syscall);
> +DECL_FILTER(fd);
> +#undef DECL_FILTER
> +
> +#define FILTER_TYPE(name) \
> +{#name, parse_ ## name ## _filter, run_ ## name ## _filter, \
> + free_ ## name ## _filter}
> +/* End of FILTER_TYPE definition. */
> +
> +static const struct filter_type {
> + const char *name;
> + void *(*parse_filter)(const char *);
> + 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++) {
> + if (!strcmp(filter_types[i].name, str))
> + 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);
> + struct filter *filter;
> +
> + if (!type)
> + error_msg_and_die("invalid filter '%s'", name);
> + *filters = xreallocarray(*filters, ++(*nfilters),
> + sizeof(struct filter));
> + filter = &((*filters)[*nfilters - 1]);
> + filter->type = type;
> + return filter;
> +}
> +
> +void
> +parse_filter(struct filter *filter, const char *str)
> +{
> + filter->_priv_data = filter->type->parse_filter(str);
> +}
> +
> +static bool
> +run_filter(struct tcb *tcp, struct filter *filter)
> +{
> + return filter->type->run_filter(tcp, filter->_priv_data);
> +}
> +
> +void
> +run_filters(struct tcb *tcp, struct filter *filters, unsigned int nfilters,
> + bool *variables_buf)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < nfilters; ++i)
> + variables_buf[i] = run_filter(tcp, &filters[i]);
> +}
> +
> +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)[0] = (*filters)[*nfilters - 1];
> + *filters = xreallocarray(*filters, 1, sizeof(struct filter));
> + *nfilters = 1;
> +}
> diff --git a/filter.h b/filter.h
> index eefbd3cb..ec61d0c4 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -29,13 +29,38 @@
> #ifndef STRACE_FILTER_H
> #define STRACE_FILTER_H
>
> -struct number_set;
> +struct filter;
> +
> +struct filter_action;
> +
> +struct bool_expression;
> +
> 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);
> +void parse_inject_common_args(char *, struct inject_opts *, const char *delim,
> + const bool fault_tokens_only);
> +
> +/* filter api */
> +struct filter* add_filter_to_array(struct filter **, unsigned int *nfilters,
> + const char *name);
> +void parse_filter(struct filter *, const char *str);
> +void run_filters(struct tcb *, struct filter *, unsigned int, bool *);
> +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);
> +
> +/* filter action api */
> +struct filter *create_filter(struct filter_action *, const char *name);
> +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 *);
> +void set_qualify_mode(struct filter_action *);
>
> -void add_number_to_set(unsigned int number, struct number_set *set);
> -void qualify_tokens(const char *str, struct number_set *set,
> - string_to_uint_func func, const char *name);
> -void qualify_syscall_tokens(const char *str, struct number_set *set,
> - const char *name);
> +/* filter expression api */
> +struct bool_expression *create_expression();
> +bool run_expression(struct bool_expression *, bool *, unsigned int);
> +void set_expression_qualify_mode(struct bool_expression *);
>
> #endif /* !STRACE_FILTER_H */
> diff --git a/filter_action.c b/filter_action.c
> new file mode 100644
> index 00000000..c040eda6
> --- /dev/null
> +++ b/filter_action.c
> @@ -0,0 +1,230 @@
> +/*
> + * 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 "filter.h"
> +
> +#define DECL_FILTER_ACTION(name) \
> +extern void \
> +apply_ ## name(struct tcb *, void *) \
> +/* End of DECL_FILTER_ACTION definition. */
> +
> +DECL_FILTER_ACTION(trace);
> +DECL_FILTER_ACTION(inject);
> +DECL_FILTER_ACTION(fault);
> +DECL_FILTER_ACTION(read);
> +DECL_FILTER_ACTION(write);
> +DECL_FILTER_ACTION(raw);
> +DECL_FILTER_ACTION(abbrev);
> +DECL_FILTER_ACTION(verbose);
> +#undef DECL_FILTER_ACTION
> +
> +extern bool is_traced(struct tcb *);
> +extern bool not_injected(struct tcb *);
> +
> +#define DECL_FILTER_ACTION_PARSER(name) \
> +extern void * \
> +parse_ ## name(const char *); \
> +extern void \
> +free_ ## name(void *) \
> +/* End of DECL_FILTER_ACTION_PARSER definition. */
> +
> +DECL_FILTER_ACTION_PARSER(null);
> +DECL_FILTER_ACTION_PARSER(inject);
> +DECL_FILTER_ACTION_PARSER(fault);
> +#undef DECL_FILTER_ACTION_PARSER
> +
> +#define FILTER_ACTION_TYPE(NAME, PRIORITY, FLAG, PARSER, PREFILTER) \
> +{#NAME, PRIORITY, FLAG, parse_ ## PARSER, free_ ## PARSER, PREFILTER, \
> + apply_ ## NAME}
> +/* End of FILTER_ACTION_TYPE definition. */
> +
> +static const struct filter_action_type {
> + const char *name;
> + /* The highest priority is 0. */
> + unsigned int priority;
> + unsigned int qual_flg;
> + 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, 0, QUAL_TRACE, null, NULL),
> + FILTER_ACTION_TYPE(inject, 1, QUAL_INJECT, inject, not_injected),
> + FILTER_ACTION_TYPE(fault, 1, QUAL_INJECT, fault, not_injected),
> + FILTER_ACTION_TYPE(read, 2, QUAL_READ, null, is_traced),
> + FILTER_ACTION_TYPE(write, 2, QUAL_WRITE, null, is_traced),
> + FILTER_ACTION_TYPE(raw, 2, QUAL_RAW, null, is_traced),
> + FILTER_ACTION_TYPE(abbrev, 2, QUAL_ABBREV, null, is_traced),
> + FILTER_ACTION_TYPE(verbose, 2, QUAL_VERBOSE, null, is_traced),
> +};
> +#undef FILTER_ACTION_TYPE
> +
> +struct filter_action {
> + /* Used to correct order of actions with the same priority. */
> + unsigned int id;
> + 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 bool *variables_buf;
> +
> +/*
> + * Compares action priorities. If actions have the same priority,
> + * uses LIFO order.
> + */
> +static int
> +compare_action_priority(const void *a, const void *b)
> +{
> + const struct filter_action *action_a = a;
> + const struct filter_action *action_b = b;
> + unsigned int priority_a = action_a->type->priority;
> + unsigned int priority_b = action_b->type->priority;
> +
> + if (priority_a != priority_b) {
> + return (priority_a < priority_b) ? -1 : 1;
> + } else {
> + return (action_a->id > action_b->id) ? -1 : 1;
> + }
> +}
> +
> +void
> +filtering_parsing_finish(void)
> +{
> + unsigned int maxfilters = 0;
> + unsigned int i;
> +
> + /* Sort actions by priority */
> + if (nfilter_actions == 0)
> + return;
> + qsort(filter_actions, nfilter_actions, sizeof(struct filter_action),
> + &compare_action_priority);
> +
> + /* Allocate variables_buf sufficient for any action */
> + for (i = 0; i < nfilter_actions; ++i) {
> + if (filter_actions[i].nfilters > maxfilters)
> + maxfilters = filter_actions[i].nfilters;
> + }
> + variables_buf = xcalloc(maxfilters, sizeof(bool));
> +}
> +
> +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) {
> + if (!strcmp(action_types[i].name, str))
> + return &action_types[i];
> + }
> + return NULL;
> +}
> +
> +static struct filter_action *
> +add_action(const struct filter_action_type *type)
> +{
> + struct filter_action *action;
> +
> + filter_actions = xreallocarray(filter_actions, ++nfilter_actions,
> + sizeof(struct filter_action));
> + action = &filter_actions[nfilter_actions - 1];
> + memset(action, 0, sizeof(*action));
> + action->id = nfilter_actions - 1;
> + 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);
> + unsigned int i;
> +
> + if (!type)
> + error_msg_and_die("invalid filter action '%s'", name);
> + /* If action takes arguments, add new action */
> + if (type->parse_args != &parse_null)
> + return add_action(type);
> +
> + for (i = 0; i < nfilter_actions; ++i) {
> + if (filter_actions[i].type == type)
> + return &filter_actions[i];
> + }
> + return add_action(type);
> +}
> +
> +static void
> +run_filter_action(struct tcb *tcp, struct filter_action *action)
> +{
> + if (action->type->prefilter && !action->type->prefilter(tcp))
> + return;
> + run_filters(tcp, action->filters, action->nfilters, variables_buf);
> + if (run_expression(action->expr, variables_buf, action->nfilters))
> + action->type->apply(tcp, action->_priv_data);
> +}
> +
> +struct filter *
> +create_filter(struct filter_action *action, const char *name)
> +{
> + return add_filter_to_array(&action->filters, &action->nfilters, name);
> +}
> +
> +void
> +set_qualify_mode(struct filter_action *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;
> +}
This function is not used.
> +
> +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 00000000..05f8903e
> --- /dev/null
> +++ b/filter_expression.c
> @@ -0,0 +1,235 @@
> +/*
> + * 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 <stdarg.h>
> +
> +struct expression_token {
> + enum token_type {
> + TOK_VARIABLE,
> + TOK_OPERATOR
> + } type;
> + union token_data {
> + unsigned int variable_id;
> + enum operator_type {
> + OP_NOT,
> + OP_AND,
> + OP_OR
> + } operator_id;
> + } data;
> +};
> +
> +struct bool_expression {
> + unsigned int ntokens;
> + struct expression_token *tokens;
> +};
> +
> +struct bool_expression *
> +create_expression(void)
> +{
> + return xcalloc(1, sizeof(struct bool_expression));
> +}
> +
> +static void
> +reallocate_expression(struct bool_expression *const expr,
> + const unsigned int new_ntokens)
> +{
> + if (!expr)
> + error_msg_and_die("invalid expression");
> + expr->tokens = xreallocarray(expr->tokens, new_ntokens,
> + sizeof(*expr->tokens));
> + if (new_ntokens > expr->ntokens)
> + memset(expr->tokens + expr->ntokens, 0,
> + sizeof(*expr->tokens) * (new_ntokens - expr->ntokens));
> + expr->ntokens = new_ntokens;
> +}
> +
> +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;
> +}
> +
> +ATTRIBUTE_FORMAT((printf, 3, 4))
> +static int
> +printf_append(char **ptr, char *end, const char *fmt, ...)
> + {
> + int ret;
> + va_list args;
> +
> + va_start(args, fmt);
> + ret = vsnprintf(*ptr, end - *ptr, fmt, args);
> + va_end(args);
> +
> + if (ret < 0)
> + return ret;
> +
> + *ptr += MIN(ret, end - *ptr);
> +
> + return ret;
Incorrect indentation.
> +}
> +
> +/* Print full diagnostics for corrupted expression */
> +static void
> +handle_corrupted_expression(struct bool_expression *expr, bool *stack,
> + unsigned int stack_size, unsigned int current_pos,
> + bool *variables, unsigned int variables_num)
> +{
> + char *buf, *pos, *end;
> + unsigned int buf_size;
> + unsigned int i;
> +
> + /* Calculate buffer size. */
> + buf_size = sizeof("corrupted filter expression:");
> + buf_size += sizeof("expression (ntokens = ):")
> + + 3 * sizeof(unsigned int)
> + + (sizeof("op_") + 3 * sizeof(int)) * expr->ntokens;
> + buf_size += sizeof("variables (nvariables = ):") + 3 * sizeof(int)
> + + sizeof("false") * variables_num;
> + buf_size += sizeof("current position: ") + 3 * sizeof(int);
> + buf_size += sizeof("stack (stack_size = ):") + 3 * sizeof(int)
> + + sizeof("false") * stack_size;
> +
> + buf = xcalloc(buf_size, 1);
> + pos = buf;
> + end = buf + buf_size;
> +
> + printf_append(&pos, end, "corrupted filter expression:\n");
> +
> + /* Print expression. */
> + printf_append(&pos, end, "expression (ntokens = %u):", expr->ntokens);
> + for (i = 0; i < expr->ntokens; ++i) {
> + switch (expr->tokens[i].type) {
> + case TOK_VARIABLE:
> + printf_append(&pos, end, " v_%u",
> + expr->tokens[i].data.variable_id);
> + break;
> + case TOK_OPERATOR:
> + switch (expr->tokens[i].data.operator_id) {
> + case OP_NOT:
> + printf_append(&pos, end, " not");
> + break;
> + case OP_AND:
> + printf_append(&pos, end, " and");
> + break;
> + case OP_OR:
> + printf_append(&pos, end, " or");
Fall through?
> + default:
> + printf_append(&pos, end, " op_%d",
> + expr->tokens[i].data.operator_id);
> + }
Fall through?
> + default:
> + printf_append(&pos, end, " ?_%d", expr->tokens[i].type);
> + }
> + }
> + printf_append(&pos, end, "\n");
> +
> + /* Print variables. */
> + printf_append(&pos, end, "variables (nvariables = %u):", variables_num);
> + for (i = 0; i < variables_num; ++i)
> + printf_append(&pos, end, variables[i] ? " true" : " false");
More like !variables[i] ? " false" : " true".
> + printf_append(&pos, end, "\n");
> +
> + printf_append(&pos, end, "current position: %u\n", current_pos);
> +
> + /* Print current stack state. */
> + printf_append(&pos, end, "stack (stack_size = %u):", stack_size);
> + for (i = 0; i < stack_size; ++i)
> + printf_append(&pos, end, stack[i] ? " true" : " false");
Same here.
> +
> + error_msg_and_die("%s", buf);
> + free(buf);
free() after error_msg_and_die() looks funny.
I think you can add ATTRIBUTE_NORETURN to the function definition.
> +}
> +
> +#define MAX_STACK_SIZE 32
> +
> +bool
> +run_expression(struct bool_expression *expr, bool *variables,
> + unsigned int variables_num)
> +{
> + bool stack[MAX_STACK_SIZE];
> + unsigned int stack_size = 0;
> + unsigned int i;
> +
> + for (i = 0; i < expr->ntokens; ++i) {
> + struct expression_token *tok = &expr->tokens[i];
> +
> + switch (tok->type) {
> + case TOK_VARIABLE:
> + if (stack_size == MAX_STACK_SIZE)
> + error_msg_and_die("stack overflow");
Btw, if you do not expect stack overflow after successful parsing, you
can call handle_corrupted_expression() here.
> +
> + if (tok->data.variable_id >= variables_num)
> + handle_corrupted_expression(expr, stack,
> + stack_size, i,
> + variables,
> + variables_num);
> + stack[stack_size++] = variables[tok->data.variable_id];
> + break;
> + case TOK_OPERATOR:
> + switch (tok->data.operator_id) {
> + case OP_NOT:
> + if (stack_size == 0)
> + handle_corrupted_expression(expr, stack,
> + stack_size, i,
> + variables,
> + variables_num);
Looking at this problem with line continuation, maybe it's better to
rename handle_corrupted_expression() to something shorted, like
"dump_expr_and_die" or "bad_expression".
> + stack[stack_size - 1] = !stack[stack_size - 1];
> + break;
> + case OP_AND:
> + if (stack_size < 2)
> + handle_corrupted_expression(expr, stack,
> + stack_size, i,
> + variables,
> + variables_num);
> + stack[stack_size - 2] = stack[stack_size - 2]
> + && stack[stack_size - 1];
> + --stack_size;
> + break;
> + case OP_OR:
> + if (stack_size < 2)
> + handle_corrupted_expression(expr, stack,
> + stack_size, i,
> + variables,
> + variables_num);
> + stack[stack_size - 2] = stack[stack_size - 2]
> + || stack[stack_size - 1];
> + --stack_size;
> + break;
I'd call handle_corrupted_expression() in default case.
> + }
Fall through?
Same here, unknown token type is a reason for colling corrupted
expression handler.
> + }
> + }
> +
> + if (stack_size != 1)
> + handle_corrupted_expression(expr, stack, stack_size, i,
> + variables, variables_num);
> + return stack[0];
> +}
> diff --git a/filter_qualify.c b/filter_qualify.c
> index 4283e769..3765f656 100644
> --- a/filter_qualify.c
> +++ b/filter_qualify.c
> @@ -38,16 +38,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
> sigstr_to_uint(const char *s)
> {
> @@ -156,174 +148,174 @@ parse_inject_token(const char *const token, struct inject_opts *const fopts,
> return true;
> }
>
> -static char *
> -parse_inject_expression(const char *const s, char **buf,
> - struct inject_opts *const fopts,
> - const bool fault_tokens_only)
> +void
> +parse_inject_common_args(char *str, struct inject_opts *const opts,
> + const char *delim, const bool fault_tokens_only)
> {
> char *saveptr = NULL;
> - char *name = NULL;
> char *token;
>
> - *buf = xstrdup(s);
> - for (token = strtok_r(*buf, ":", &saveptr); token;
> - token = strtok_r(NULL, ":", &saveptr)) {
> - if (!name)
> - name = token;
> - else if (!parse_inject_token(token, fopts, fault_tokens_only))
> - goto parse_error;
> - }
> + opts->first = 1;
> + opts->step = 1;
> + opts->rval = INJECT_OPTS_RVAL_DEFAULT;
> + opts->signo = 0;
> + opts->init = false;
>
> - if (name)
> - return name;
> + for (token = strtok_r(str, delim, &saveptr); token;
> + token = strtok_r(NULL, delim, &saveptr)) {
> + if (!parse_inject_token(token, opts, fault_tokens_only))
> + return;
> + }
>
> -parse_error:
> - free(*buf);
> - return *buf = NULL;
> + /* If neither of retval, error, or signal is specified, then ... */
> + if (opts->rval == INJECT_OPTS_RVAL_DEFAULT && !opts->signo) {
> + if (fault_tokens_only) {
> + /* in fault= syntax the default error code is ENOSYS. */
> + opts->rval = -ENOSYS;
> + } else {
> + /* in inject= syntax this is not allowed. */
> + return;
> + }
> + }
> + opts->init = true;
> }
>
> 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);
> + 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);
> + set_qualify_mode(action);
> }
>
> static void
> qualify_signals(const char *const str)
> {
> - qualify_tokens(str, &signal_set, sigstr_to_uint, "signal");
> + /* Clear the set. */
> + if (signal_set.nslots)
> + memset(signal_set.vec, 0,
> + sizeof(*signal_set.vec) * signal_set.nslots);
> + signal_set.not = false;
> +
> + 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);
> + 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);
> + 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);
> + 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);
> + set_qualify_mode(action);
> }
>
> static void
> -qualify_inject_common(const char *const str,
> - const bool fault_tokens_only,
> - const char *const description)
> +parse_inject_common_qualify(const char *const str, const bool fault_tokens_only,
> + const char *const description)
> {
> - struct inject_opts opts = {
> - .first = 1,
> - .step = 1,
> - .rval = INJECT_OPTS_RVAL_DEFAULT,
> - .signo = 0
> - };
> - char *buf = NULL;
> - char *name = parse_inject_expression(str, &buf, &opts, fault_tokens_only);
> - if (!name) {
> - error_msg_and_die("invalid %s '%s'", description, str);
> - }
> -
> - /* If neither of retval, error, or signal is specified, then ... */
> - if (opts.rval == INJECT_OPTS_RVAL_DEFAULT && !opts.signo) {
> - if (fault_tokens_only) {
> - /* in fault= syntax the default error code is ENOSYS. */
> - opts.rval = -ENOSYS;
> - } else {
> - /* in inject= syntax this is not allowed. */
> - error_msg_and_die("invalid %s '%s'", description, str);
> - }
> - }
> -
> - struct number_set tmp_set[SUPPORTED_PERSONALITIES];
> - memset(tmp_set, 0, sizeof(tmp_set));
> - qualify_syscall_tokens(name, tmp_set, description);
> -
> + struct inject_opts *opts = xmalloc(sizeof(struct inject_opts));
> + char *buf = xstrdup(str);
> + struct filter_action *action;
> + struct filter *filter;
> + char *args = strchr(buf, ':');
> +
> + if (args)
> + *(args++) = '\0';
> +
> + action = find_or_add_action(fault_tokens_only ? "fault" : "inject");
> + filter = create_filter(action, "syscall");
> + parse_filter(filter, buf);
> + set_qualify_mode(action);
> + parse_inject_common_args(args, opts, ":", fault_tokens_only);
> + if (!opts->init)
> + error_msg_and_die("invalid %s '%s'", description,
> + args ? args : "");
> free(buf);
> -
> - /*
> - * Initialize inject_vec accourding to tmp_set.
> - * Merge tmp_set into inject_set.
> - */
> - unsigned int p;
> - for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> - if (!tmp_set[p].nslots && !tmp_set[p].not) {
> - continue;
> - }
> -
> - if (!inject_vec[p]) {
> - inject_vec[p] = xcalloc(nsyscall_vec[p],
> - sizeof(*inject_vec[p]));
> - }
> -
> - 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, opts);
> }
>
> static void
> -qualify_fault(const char *const str)
> +parse_fault(const char *const str)
> {
> - qualify_inject_common(str, true, "fault argument");
> + parse_inject_common_qualify(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_qualify(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;
> @@ -342,18 +334,3 @@ qualify(const char *str)
>
> 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 ae93f923..88f7168e 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -770,7 +770,8 @@ droptcb(struct tcb *tcp)
>
> int p;
> for (p = 0; p < SUPPORTED_PERSONALITIES; ++p)
> - free(tcp->inject_vec[p]);
> + if (tcp->inject_vec[p])
> + free(tcp->inject_vec[p]);
>
> free_tcb_priv_data(tcp);
>
> @@ -1576,13 +1577,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
> @@ -1649,7 +1650,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();
> @@ -1664,7 +1665,7 @@ init(int argc, char *argv[])
> error_opt_arg(c, optarg);
> break;
> case 'e':
> - qualify(optarg);
> + parse_qualify_filter(optarg);
> break;
> case 'o':
> outfname = optarg;
> @@ -1712,6 +1713,7 @@ init(int argc, char *argv[])
> break;
> }
> }
> + filtering_parsing_finish();
>
> argv += optind;
> argc -= optind;
> @@ -2414,6 +2416,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 a0c649ea..b740dadb 100644
> --- a/syscall.c
> +++ b/syscall.c
> @@ -345,7 +345,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;
> @@ -385,7 +384,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;
> @@ -402,7 +400,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]));
> @@ -431,7 +428,7 @@ dumpio(struct tcb *tcp)
> if (fd < 0)
> return;
>
> - if (is_number_in_set(fd, &read_set)) {
> + if (dump_read(tcp)) {
> switch (tcp->s_ent->sen) {
> case SEN_read:
> case SEN_pread:
> @@ -454,7 +451,7 @@ dumpio(struct tcb *tcp)
> return;
> }
> }
> - if (is_number_in_set(fd, &write_set)) {
> + if (dump_write(tcp)) {
> switch (tcp->s_ent->sen) {
> case SEN_write:
> case SEN_pwrite:
> @@ -540,8 +537,6 @@ 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)
> {
> @@ -553,14 +548,6 @@ tcb_inject_opts(struct tcb *tcp)
> 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));
> - }
> -
> struct inject_opts *opts = tcb_inject_opts(tcp);
>
> if (!opts || opts->first == 0)
> @@ -680,9 +667,7 @@ syscall_entering_trace(struct tcb *tcp, unsigned int *sig)
> break;
> }
>
> - if (!(tcp->qual_flg & QUAL_TRACE)
> - || (tracing_paths && !pathtrace_match(tcp))
> - ) {
> + if (!(tcp->qual_flg & QUAL_TRACE)) {
Btw, we can used "traced(tcp)" or something like this for this check,
similarly to other qual_flg checks.
> tcp->flags |= TCB_FILTERED;
> return 0;
> }
> @@ -1201,7 +1186,8 @@ get_scno(struct tcb *tcp)
>
> if (scno_is_valid(tcp->scno)) {
> tcp->s_ent = &sysent[tcp->scno];
> - tcp->qual_flg = qual_flags(tcp->scno);
> + /* Clear qual_flg to differ valid syscall from printargs */
> + tcp->qual_flg = 0;
> } else {
> struct sysent_buf *s = xcalloc(1, sizeof(*s));
>
> diff --git a/tests/filtering_syscall-syntax.test b/tests/filtering_syscall-syntax.test
> index 698a894c..2a4125ae 100755
> --- a/tests/filtering_syscall-syntax.test
> +++ b/tests/filtering_syscall-syntax.test
> @@ -35,10 +35,12 @@ check_syscall()
> check_e "invalid system call '$1'" -e abbrev="$2"
> check_e "invalid system call '$1'" -e verbose="$2"
> check_e "invalid system call '$1'" -e raw="$2"
> + check_e "invalid system call '$1'" -e inject="$2"
> + check_e "invalid system call '$1'" -e fault="$2"
> }
>
> # invalid syscall
> -for arg in '' , ,, ,,, : :: ::: \! \!, \
> +for arg in '' , ,, ,,, \! \!, \
> invalid_syscall_name \
> -1 -2 -3 -4 -5 \
> 32767 \
> @@ -52,6 +54,25 @@ for arg in '' , ,, ,,, : :: ::: \! \!, \
> check_syscall "$arg" "$arg"
> done
>
> +# Temporary workaround
> +for arg in : :: ::: \
> + ; do
> + check_e "invalid system call '$arg'" -e trace="$arg"
> + check_e "invalid system call '$arg'" -e abbrev="$arg"
> + check_e "invalid system call '$arg'" -e verbose="$arg"
> + check_e "invalid system call '$arg'" -e raw="$arg"
> + check_e "invalid system call ''" -e inject="$arg"
> + check_e "invalid system call ''" -e fault="$arg"
> +done
> +
> +# Temporary workaround for '!:' check
> +check_e "invalid system call ':'" -e trace=\!:
> +check_e "invalid system call ':'" -e abbrev=\!:
> +check_e "invalid system call ':'" -e verbose=\!:
> +check_e "invalid system call ':'" -e raw=\!:
> +check_e "invalid system call '!'" -e inject=\!:
> +check_e "invalid system call '!'" -e fault=\!:
> +
> # invalid syscall with inversion
> for arg in -1 -2 -3 -4 -5 \
> invalid_syscall_name \
> @@ -78,7 +99,6 @@ for arg in file,nonsense \
> done
>
> # special cases
> -check_syscall ":" \!:
> check_syscall "!" 1,\!
> check_syscall "!open" 1,\!open
> check_syscall "none" 1,none
> diff --git a/tests/qual_fault-syntax.test b/tests/qual_fault-syntax.test
> index 0cce539a..5bc58194 100755
> --- a/tests/qual_fault-syntax.test
> +++ b/tests/qual_fault-syntax.test
> @@ -27,7 +27,7 @@
> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> # THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>
> -. "${srcdir=.}/init.sh"
> +. "${srcdir=.}/syntax.sh"
>
> #
> # F
> @@ -40,16 +40,7 @@ fail_with()
> "strace -e fault=$* failed to handle an argument error properly"
> }
>
> -for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> - invalid_syscall_name \
> - invalid_syscall_name:when=3 \
> - -1 \!-1 \
> - -1:when=4 \
> - -2 \
> - -2:when=5 \
> - 32767 \!32767 \
> - 32767:when=6 \
> - chdir:42 \!chdir:42 \
I do not see where these tests are moved to.
> +for arg in chdir:42 \!chdir:42 \
> chdir:42:when=7 \
> chdir:invalid \
> chdir:invalid:when=8 \
> @@ -92,12 +83,6 @@ for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> chdir:when=65536:error=30 \
> chdir:when=1+65536 \
> chdir:when=1+65536:error=31 \
> - file,nonsense \
> - \!desc,nonsense \
> - chdir,nonsense \
> - \!chdir,nonsense \
> - 1,nonsense \
> - \!1,nonsense \
I do not see where these tests are moved to.
> chdir:retval=0 \
> chdir:signal=1 \
> chdir:error=1:error=2 \
> @@ -108,4 +93,11 @@ for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> fail_with "$arg"
> done
>
> +# check syscall syntax with fault args
> +for arg in invalid_syscall_name \
> + -1 -2 32767 \
> + ; do
> + check_e "invalid system call '$arg'" -e fault="$arg:when=1"
> +done
> +
> exit 0
> diff --git a/tests/qual_inject-syntax.test b/tests/qual_inject-syntax.test
> index a9e44d74..7859d825 100755
> --- a/tests/qual_inject-syntax.test
> +++ b/tests/qual_inject-syntax.test
> @@ -27,7 +27,7 @@
> # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> # THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>
> -. "${srcdir=.}/init.sh"
> +. "${srcdir=.}/syntax.sh"
>
> #
> # F
> @@ -40,17 +40,7 @@ fail_with()
> "strace -e inject=$* failed to handle an argument error properly"
> }
>
> -for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> - invalid_syscall_name \
> - invalid_syscall_name:when=3 \
> - -1 \!-1 \
> - -1:when=4 \
> - -2 \
> - -2:when=5 \
> - 32767 \!32767 \
> - 32767:when=6 \
> - 42 \
> - chdir \
I do not see where these tests are moved to.
> +for arg in 42 chdir \
> chdir:42 \!chdir:42 \
> chdir:42:when=7 \
> chdir:invalid \
> @@ -94,12 +84,6 @@ for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> chdir:when=65536:error=30 \
> chdir:when=1+65536 \
> chdir:when=1+65536:error=31 \
> - file,nonsense \
> - \!desc,nonsense \
> - chdir,nonsense \
> - \!chdir,nonsense \
> - 1,nonsense \
> - \!1,nonsense \
I do not see where these tests are moved to.
> chdir:retval=-1 \
> chdir:signal=0 \
> chdir:signal=129 \
> @@ -115,4 +99,11 @@ for arg in '' , ,, ,,, : :: ::: \! \!, \!: \
> fail_with "$arg"
> done
>
> +# check syscall syntax with inject args
> +for arg in invalid_syscall_name \
> + -1 -2 32767 \
> + ; do
> + check_e "invalid system call '$arg'" -e inject="$arg:when=1"
> +done
> +
> exit 0
Regarding test coverage (this is not exclusive to this patch, but the overall
patch set): lcov shows that there are significant omissions in regards the
code paths tested. Among those:
* Not all branches of is_allowed_in_name are covered. The function call
count (4) implies it's almost hasn't been touched.
* No check that hist stack overflow during the evaluation
* Several pretty hittable paths in push_operator, is_higher_priority,
parse_filter_expression are not hit
* Some branches of filtering_parse are not covered (specifically, case
'(' in case F_EMPTY, case F_FILT_ARGS, case F_END).
* Not all branches of is_space_ascii are covered, which also indicates
* that related code isn't tortured enough.
* prase_inject, free_inject, parse_fault, free_fault,
parse_inject_common from basic_actions.c (again, for function name
collision) haven't been called even once.
* lookup_filter_type never returned NULL, or even failed to find
requested type. Same for lookup_filter_action_type.
* get_filter_priv_data/set_filter_priv_data also haven't been called
ever during the execution of the test suite, but I assume it's also
due to the fact they are used by injection filter only.
* parse_syscall_regex never fed with incorrect regular expression.
* {parse,run,free}_path_filter functions are also untouched.
* Errors "... action takes no arguments" are not hit.
* Path for invalid qualify action is not hit.
The coverage report for my system is in attachment (hope I will not
forget to attach it this time).
> --
> 2.11.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: strace-4.18.0.195.c09c4-coverage.tar.xz
Type: application/x-xz
Size: 381440 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170814/d87c51cf/attachment.bin>
More information about the Strace-devel
mailing list