[PATCH v5 4/8] Improve fd filtering

Eugene Syromiatnikov esyr at redhat.com
Sun Jul 23 15:59:59 UTC 2017


On Wed, Jul 19, 2017 at 05:26:47PM +0700, Nikolay Marchuk wrote:
> * pathtrace.c (match_fd_common, pathtrace_match_set): Move fd matching to
> separate function.
> * filter.h (match_fd_common): Add new declaration.
> * basic_filters.c (run_fd_filter): Use match_fd_common for fd filter.
> ---
>  basic_filters.c |  53 +++++++++++++--
>  filter.h        |   2 +
>  pathtrace.c     | 198 ++++++++++++++++++++++++++++++++------------------------
>  3 files changed, 166 insertions(+), 87 deletions(-)
> 
> diff --git a/basic_filters.c b/basic_filters.c
> index 80cbf67..ca4f78c 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -29,6 +29,7 @@
>  #include "defs.h"
>  #include <regex.h>
>  #include "filter.h"
> +#include "syscall.h"
>  
>  typedef unsigned int number_slot_t;
>  #define BITS_PER_SLOT (sizeof(number_slot_t) * 8)
> @@ -410,14 +411,58 @@ parse_fd_filter(const char *str)
>  	return set;
>  }
>  
> +static bool
> +is_fd_in_set(struct tcb *tcp, int fd, void *data) {
> +	if (fd < 0)
> +		return false;
> +	struct number_set *set = data;
> +	return is_number_in_set(fd, 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 = _priv_data;
> -	return is_number_in_set(fd, set);
> +	const struct_sysent *s_ent = tcp->s_ent;
> +	/*
> +	 * mq_timedsend and mq_timedreceive are not marked as descriptor
> +	 * syscalls, but they can be dumped with -e read/write.
> +	*/
> +	switch (s_ent->sen) {
> +	case SEN_mq_timedsend:
> +	case SEN_mq_timedreceive:
> +		return is_fd_in_set(tcp, tcp->u_arg[0], set);
> +	}
> +
> +	if (!(s_ent->sys_flags & (TRACE_DESC | TRACE_NETWORK)))
> +		return false;
> +
> +	int res;
> +	if ((res = match_fd_common(tcp, &is_fd_in_set, set)) != -1)
> +		return res ? true : false;
> +
> +	switch (s_ent->sen) {
> +	case SEN_bpf:
> +	case SEN_creat:
> +	case SEN_epoll_create:
> +	case SEN_epoll_create1:
> +	case SEN_eventfd2:
> +	case SEN_eventfd:
> +	case SEN_fanotify_init:
> +	case SEN_inotify_init1:
> +	case SEN_memfd_create:
> +	case SEN_open:
> +	case SEN_perf_event_open:
> +	case SEN_pipe:
> +	case SEN_pipe2:
> +	case SEN_printargs:
> +	case SEN_socket:
> +	case SEN_socketpair:
> +	case SEN_timerfd_create:
> +	case SEN_userfaultfd:
> +		return false;
> +	}
> +	return is_fd_in_set(tcp, tcp->u_arg[0], set);
>  }
>  
>  void
> diff --git a/filter.h b/filter.h
> index 7525b15..c420fe6 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -41,6 +41,8 @@ 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);
> +typedef bool (*match_fd_func)(struct tcb *, int, void *);
> +int match_fd_common(struct tcb *, match_fd_func, void *);
>  
>  /* filter api */
>  struct filter* add_filter_to_array(struct filter **, unsigned int *nfilters,
> diff --git a/pathtrace.c b/pathtrace.c
> index 9f3674a..0c896cc 100644
> --- a/pathtrace.c
> +++ b/pathtrace.c
> @@ -69,6 +69,8 @@ upathmatch(struct tcb *const tcp, const kernel_ulong_t upath,
>  static bool
>  fdmatch(struct tcb *tcp, int fd, struct path_set *set)
>  {
> +	if (fd < 0)
> +		return false;
>  	char path[PATH_MAX + 1];
>  	int n = getfdpath(tcp, fd, path, sizeof(path));
>  
> @@ -143,25 +145,26 @@ pathtrace_select_set(const char *path, struct path_set *set)
>  	storepath(rpath, set);
>  }
>  
> -/*
> - * Return true if syscall accesses a selected path
> - * (or if no paths have been specified for tracing).
> - */
> -bool
> -pathtrace_match_set(struct tcb *tcp, struct path_set *set)
> -{
> -	const struct_sysent *s;
> -
> -	s = tcp->s_ent;
> -
> -	if (!(s->sys_flags & (TRACE_FILE | TRACE_DESC | TRACE_NETWORK)))
> -		return false;
> +typedef bool (*match_fd_func)(struct tcb *, int, void *);
>  
> -	/*
> -	 * Check for special cases where we need to do something
> -	 * other than test arg[0].
> -	 */
> +static
> +bool fdmatch_fd_func(struct tcb *tcp, int fd, void *data)
> +{
> +	return fdmatch(tcp, fd, (struct path_set *) data);
> +}
>  
> +/* Match fd with func.
> + * Return values:
> + * 0	fd rejected,
> + * 1	fd accepted,
> + * -1	additional checks required.
> + */
> +int
> +match_fd_common(struct tcb *tcp, match_fd_func func, void *data)
> +{
> +	const struct_sysent *s = tcp->s_ent;
> +	if (!(s->sys_flags & (TRACE_DESC | TRACE_NETWORK)))
> +		return -1;
And what about all the syscalls with one of TD|TN that are not present in
the following switch statement, like inotify_add_watch?

>  	switch (s->sen) {
>  	case SEN_dup2:
>  	case SEN_dup3:
> @@ -170,47 +173,17 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  	case SEN_sendfile64:
>  	case SEN_tee:
>  		/* fd, fd */
> -		return fdmatch(tcp, tcp->u_arg[0], set) ||
> -			fdmatch(tcp, tcp->u_arg[1], set);
> -
> -	case SEN_faccessat:
> -	case SEN_fchmodat:
> -	case SEN_fchownat:
> -	case SEN_fstatat64:
> -	case SEN_futimesat:
> -	case SEN_inotify_add_watch:
> -	case SEN_mkdirat:
> -	case SEN_mknodat:
> -	case SEN_name_to_handle_at:
> -	case SEN_newfstatat:
> -	case SEN_openat:
> -	case SEN_readlinkat:
> -	case SEN_statx:
> -	case SEN_unlinkat:
> -	case SEN_utimensat:
> -		/* fd, path */
> -		return fdmatch(tcp, tcp->u_arg[0], set) ||
> -			upathmatch(tcp, tcp->u_arg[1], set);
> -
> -	case SEN_link:
> -	case SEN_mount:
> -	case SEN_pivotroot:
> -		/* path, path */
> -		return upathmatch(tcp, tcp->u_arg[0], set) ||
> -			upathmatch(tcp, tcp->u_arg[1], set);
> -
> -	case SEN_quotactl:
> -		/* x, path */
> -		return upathmatch(tcp, tcp->u_arg[1], set);
> +		return func(tcp, tcp->u_arg[0], data) ||
> +			func(tcp, tcp->u_arg[1], data) ? 1 : 0;
>  
>  	case SEN_linkat:
>  	case SEN_renameat2:
>  	case SEN_renameat:
> -		/* fd, path, fd, path */
> -		return fdmatch(tcp, tcp->u_arg[0], set) ||
> -			fdmatch(tcp, tcp->u_arg[2], set) ||
> -			upathmatch(tcp, tcp->u_arg[1], set) ||
> -			upathmatch(tcp, tcp->u_arg[3], set);
> +	case SEN_copy_file_range:
> +	case SEN_splice:
> +		/* fd, x, fd */
> +		return func(tcp, tcp->u_arg[0], data) ||
> +			func(tcp, tcp->u_arg[2], data) ? 1 : 0;
>  
>  	case SEN_old_mmap:
>  #if defined(S390)
> @@ -221,30 +194,21 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  	case SEN_mmap_pgoff:
>  	case SEN_ARCH_mmap:
>  		/* x, x, x, x, fd */
> -		return fdmatch(tcp, tcp->u_arg[4], set);
> -
> +		return func(tcp, tcp->u_arg[4], data) ? 1 : 0;
>  	case SEN_symlinkat:
> -		/* path, fd, path */
> -		return fdmatch(tcp, tcp->u_arg[1], set) ||
> -			upathmatch(tcp, tcp->u_arg[0], set) ||
> -			upathmatch(tcp, tcp->u_arg[2], set);
> -
> -	case SEN_copy_file_range:
> -	case SEN_splice:
> -		/* fd, x, fd, x, x, x */
> -		return fdmatch(tcp, tcp->u_arg[0], set) ||
> -			fdmatch(tcp, tcp->u_arg[2], set);
> -
> +		/* x, fd, x */
> +		return func(tcp, tcp->u_arg[1], data) ? 1 : 0;
>  	case SEN_epoll_ctl:
>  		/* x, x, fd, x */
> -		return fdmatch(tcp, tcp->u_arg[2], set);
> -
> +		return func(tcp, tcp->u_arg[2], data) ? 1 : 0;
>  
>  	case SEN_fanotify_mark:
> -		/* x, x, x, fd, path */
> -		return fdmatch(tcp, tcp->u_arg[3], set) ||
> -			upathmatch(tcp, tcp->u_arg[4], set);
> -
> +	{
> +		/* x, x, x, fd, x */
> +		unsigned long long mask = 0;
> +		int argn = getllval(tcp, &mask, 2);
> +		return func(tcp, tcp->u_arg[argn], data) ? 1 : 0;
> +	}
>  	case SEN_oldselect:
>  	case SEN_pselect6:
>  	case SEN_select:
> @@ -260,13 +224,13 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  		if (SEN_oldselect == s->sen) {
>  			if (sizeof(*select_args) == sizeof(*oldselect_args)) {
>  				if (umove(tcp, tcp->u_arg[0], &select_args)) {
> -					return false;
> +					return 0;
>  				}
>  			} else {
>  				unsigned int n;
>  
>  				if (umove(tcp, tcp->u_arg[0], &oldselect_args)) {
> -					return false;
> +					return 0;
>  				}
>  
>  				for (n = 0; n < 5; ++n) {
> @@ -282,7 +246,7 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  		nfds = (int) args[0];
>  		/* Kernel rejects negative nfds, so we don't parse it either. */
>  		if (nfds <= 0)
> -			return false;
> +			return 0;
>  		/* Beware of select(2^31-1, NULL, NULL, NULL) and similar... */
>  		if (nfds > 1024*1024)
>  			nfds = 1024*1024;
> @@ -299,14 +263,14 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  				j = next_set_bit(fds, j, nfds);
>  				if (j < 0)
>  					break;
> -				if (fdmatch(tcp, j, set)) {
> +				if (func(tcp, j, data)) {
>  					free(fds);
> -					return true;
> +					return 1;
>  				}
>  			}
>  		}
>  		free(fds);
> -		return false;
> +		return 0;
>  	}
>  
>  	case SEN_poll:
> @@ -322,15 +286,83 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
>  		end = start + sizeof(fds) * nfds;
>  
>  		if (nfds == 0 || end < start)
> -			return false;
> +			return 0;
>  
>  		for (cur = start; cur < end; cur += sizeof(fds))
>  			if ((umove(tcp, cur, &fds) == 0)
> -			    && fdmatch(tcp, fds.fd, set))
> -				return true;
> +			    && func(tcp, fds.fd, data))
> +				return 1;
>  
> -		return false;
> +		return 0;
>  	}
> +	}
> +	return -1;
> +}
> +
> +/*
> + * Return true if syscall accesses a selected path
> + * (or if no paths have been specified for tracing).
> + */
> +bool
> +pathtrace_match_set(struct tcb *tcp, struct path_set *set)
> +{
> +	const struct_sysent *s;
> +
> +	s = tcp->s_ent;
> +
> +	if (!(s->sys_flags & (TRACE_FILE | TRACE_DESC | TRACE_NETWORK)))
> +		return false;
> +
> +	int res;
> +	if ((res = match_fd_common(tcp, fdmatch_fd_func, set)) != -1)
> +		return res ? true : false;
> +
> +	/*
> +	 * Check for special cases where we need to do something
> +	 * other than test arg[0].
> +	 */
> +	switch (s->sen) {
> +	case SEN_faccessat:
> +	case SEN_fchmodat:
> +	case SEN_fchownat:
> +	case SEN_fstatat64:
> +	case SEN_futimesat:
> +	case SEN_inotify_add_watch:
> +	case SEN_mkdirat:
> +	case SEN_mknodat:
> +	case SEN_name_to_handle_at:
> +	case SEN_newfstatat:
> +	case SEN_openat:
> +	case SEN_quotactl:
> +	case SEN_readlinkat:
> +	case SEN_statx:
> +	case SEN_unlinkat:
> +	case SEN_utimensat:
> +		/* x, path */
> +		return upathmatch(tcp, tcp->u_arg[1], set);
So, in case fd matches and path doesn't, this check simply fails in case
of inotify_add_watch.

> +
> +	case SEN_link:
> +	case SEN_mount:
> +	case SEN_pivotroot:
> +		/* path, path */
> +		return upathmatch(tcp, tcp->u_arg[0], set) ||
> +			upathmatch(tcp, tcp->u_arg[1], set);
> +
> +	case SEN_linkat:
> +	case SEN_renameat2:
> +	case SEN_renameat:
> +		/* x, path, x, path */
> +		return upathmatch(tcp, tcp->u_arg[1], set) ||
> +			upathmatch(tcp, tcp->u_arg[3], set);
> +
> +	case SEN_symlinkat:
> +		/* path, x, path */
> +		return upathmatch(tcp, tcp->u_arg[0], set) ||
> +			upathmatch(tcp, tcp->u_arg[2], set);
> +
> +	case SEN_fanotify_mark:
> +		/* x, x, x, x, path */
> +		return upathmatch(tcp, tcp->u_arg[4], set);
>  
>  	case SEN_bpf:
>  	case SEN_epoll_create:
> -- 
> 2.1.4




More information about the Strace-devel mailing list