Improve fd filtering

* pathtrace.c (fdmatch_fd_func): New function.
(match_fd_common): Likewise.
(pathtrace_match_set): Move fd matching to match_fd_common.
* filter.h (match_fd_common): Add new declaration.
* basic_filters.c (is_fd_in_set): New function.
(run_fd_filter): Use match_fd_common for fd filter.

[ldv: cleanup run_fd_filter]

3 files changed, 129 insertions, 93 deletions

diff --git a/basic_filters.c b/basic_filters.c
index 3a601ef86..3b6bed4f8 100644
--- a/basic_filters.c
+++ b/basic_filters.c
@@ -342,19 +342,21 @@ parse_fd_filter(const char *str)
 	return set;
 }
 
+static bool
+is_fd_in_set(struct tcb *tcp, int fd, void *data) {
+	return fd < 0 ? false : is_number_in_set(fd, data);
+}
+
 bool
 run_fd_filter(struct tcb *tcp, void *priv_data)
 {
-	int fd = tcp->u_arg[0];
-
-	return fd < 0 ? false : is_number_in_set(fd, priv_data);
+	return match_fd_common(tcp, &is_fd_in_set, priv_data);
 }
 
 void
 free_fd_filter(void *priv_data)
 {
 	free_number_set_array(priv_data, 1);
-	return;
 }
 
 void *
@@ -369,9 +371,7 @@ parse_path_filter(const char *path)
 bool
 run_path_filter(struct tcb *tcp, void *priv_data)
 {
-	struct path_set *set = priv_data;
-
-	return pathtrace_match_set(tcp, set);
+	return pathtrace_match_set(tcp, priv_data);
 }
 
 void
diff --git a/filter.h b/filter.h
index d9a2af751..4fdedbe71 100644
--- a/filter.h
+++ b/filter.h
@@ -47,6 +47,9 @@ void parse_inject_common_args(char *, struct inject_opts *,
 bool is_traced(struct tcb *);
 bool not_injected(struct tcb *);
 
+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,
 				   const char *name);
diff --git a/pathtrace.c b/pathtrace.c
index 45b829758..77595b967 100644
--- a/pathtrace.c
+++ b/pathtrace.c
@@ -70,6 +70,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,9 +145,17 @@ pathtrace_select_set(const char *path, struct path_set *set)
 	storepath(rpath, set);
 }
 
+typedef bool (*match_fd_func)(struct tcb *, int, void *);
+
+static
+bool fdmatch_fd_func(struct tcb *tcp, int fd, void *data)
+{
+	return fdmatch(tcp, fd, (struct path_set *) data);
+}
+
 static bool
-match_xselect_args(struct tcb *tcp, const kernel_ulong_t *args,
-		   struct path_set *set)
+match_xselect_args(struct tcb *tcp, match_fd_func func,
+		   const kernel_ulong_t *args, struct path_set *set)
 {
 	/* Kernel truncates arg[0] to int, we do the same. */
 	int nfds = (int) args[0];
@@ -167,7 +177,7 @@ match_xselect_args(struct tcb *tcp, const kernel_ulong_t *args,
 			j = next_set_bit(fds, j, nfds);
 			if (j < 0)
 				break;
-			if (fdmatch(tcp, j, set)) {
+			if (func(tcp, j, set)) {
 				free(fds);
 				return true;
 			}
@@ -178,25 +188,15 @@ match_xselect_args(struct tcb *tcp, const kernel_ulong_t *args,
 	return false;
 }
 
-/*
- * Return true if syscall accesses a selected path
- * (or if no paths have been specified for tracing).
- */
+/* Match fd with func. */
 bool
-pathtrace_match_set(struct tcb *tcp, struct path_set *set)
+match_fd_common(struct tcb *tcp, match_fd_func func, void *data)
 {
-	const struct_sysent *s;
-
-	s = tcp->s_ent;
+	const struct_sysent *s = tcp->s_ent;
 
-	if (!(s->sys_flags & (TRACE_FILE | TRACE_DESC | TRACE_NETWORK)))
+	if (!(s->sys_flags & (TRACE_DESC | TRACE_NETWORK)))
 		return false;
 
-	/*
-	 * Check for special cases where we need to do something
-	 * other than test arg[0].
-	 */
-
 	switch (s->sen) {
 	case SEN_dup2:
 	case SEN_dup3:
@@ -205,49 +205,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_execveat:
-	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:
-	case SEN_symlink:
-		/* 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);
 
+	case SEN_copy_file_range:
 	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_splice:
+		/* fd, x, fd */
+		return func(tcp, tcp->u_arg[0], data) ||
+			func(tcp, tcp->u_arg[2], data);
 
 #if HAVE_ARCH_OLD_MMAP
 	case SEN_old_mmap:
@@ -258,7 +226,7 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
 		kernel_ulong_t *args =
 			fetch_indirect_syscall_args(tcp, tcp->u_arg[0], 6);
 
-		return args && fdmatch(tcp, args[4], set);
+		return args && func(tcp, args[4], data);
 	}
 #endif /* HAVE_ARCH_OLD_MMAP */
 
@@ -267,22 +235,15 @@ 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);
 
 	case SEN_symlinkat:
-		/* x, fd, path */
-		return fdmatch(tcp, tcp->u_arg[1], 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);
 
 	case SEN_epoll_ctl:
 		/* x, x, fd, x */
-		return fdmatch(tcp, tcp->u_arg[2], set);
+		return func(tcp, tcp->u_arg[2], data);
 
 
 	case SEN_fanotify_mark:
@@ -290,21 +251,22 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
 		/* x, x, mask (64 bit), fd, path */
 		unsigned long long mask = 0;
 		int argn = getllval(tcp, &mask, 2);
-		return fdmatch(tcp, tcp->u_arg[argn], set) ||
-			upathmatch(tcp, tcp->u_arg[argn + 1], set);
+
+		return func(tcp, tcp->u_arg[argn], data);
 	}
+
 #if HAVE_ARCH_OLD_SELECT
 	case SEN_oldselect:
 	{
 		kernel_ulong_t *args =
 			fetch_indirect_syscall_args(tcp, tcp->u_arg[0], 5);
 
-		return args && match_xselect_args(tcp, args, set);
+		return args && match_xselect_args(tcp, func, args, data);
 	}
 #endif
 	case SEN_pselect6:
 	case SEN_select:
-		return match_xselect_args(tcp, tcp->u_arg, set);
+		return match_xselect_args(tcp, func, tcp->u_arg, data);
 	case SEN_poll:
 	case SEN_ppoll:
 	{
@@ -325,16 +287,21 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
 		for (cur = start; cur < end; cur += sizeof(fds)) {
 			if (umove(tcp, cur, &fds))
 				break;
-			if (fdmatch(tcp, fds.fd, set))
+			if (func(tcp, fds.fd, data))
 				return true;
 		}
 
 		return false;
 	}
 
+	/*
+	 * These have TRACE_DESCRIPTOR or TRACE_NETWORK set,
+	 * but they don't have any file descriptor to test.
+	 */
 	case SEN_accept4:
 	case SEN_accept:
 	case SEN_bpf:
+	case SEN_creat:
 	case SEN_epoll_create:
 	case SEN_epoll_create1:
 	case SEN_eventfd2:
@@ -346,8 +313,7 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
 	case SEN_mq_getsetattr:
 	case SEN_mq_notify:
 	case SEN_mq_open:
-	case SEN_mq_timedreceive:
-	case SEN_mq_timedsend:
+	case SEN_open:
 	case SEN_perf_event_open:
 	case SEN_pipe:
 	case SEN_pipe2:
@@ -360,23 +326,90 @@ pathtrace_match_set(struct tcb *tcp, struct path_set *set)
 	case SEN_timerfd_gettime:
 	case SEN_timerfd_settime:
 	case SEN_userfaultfd:
-		/*
-		 * These have TRACE_FILE or TRACE_DESCRIPTOR or TRACE_NETWORK set,
-		 * but they don't have any file descriptor or path args to test.
-		 */
 		return false;
 	}
+	return func(tcp, tcp->u_arg[0], data);
+}
 
+/*
+ * 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;
+
+	if (match_fd_common(tcp, fdmatch_fd_func, set))
+		return true;
+
+	if (!(s->sys_flags & TRACE_FILE))
+		return false;
 	/*
-	 * Our fallback position for calls that haven't already
-	 * been handled is to just check arg[0].
+	 * Check for special cases where we need to do something
+	 * other than test arg[0].
 	 */
+	switch (s->sen) {
+	case SEN_execveat:
+	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_symlink:
+	case SEN_statx:
+	case SEN_unlinkat:
+	case SEN_utimensat:
+		/* x, path */
+		return 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);
 
-	if (s->sys_flags & TRACE_FILE)
-		return upathmatch(tcp, tcp->u_arg[0], 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);
 
-	if (s->sys_flags & (TRACE_DESC | TRACE_NETWORK))
-		return fdmatch(tcp, tcp->u_arg[0], set);
+	case SEN_symlinkat:
+		/* x, x, path */
+		return upathmatch(tcp, tcp->u_arg[2], set);
 
-	return false;
+	case SEN_fanotify_mark:
+	{
+		/* x, x, mask (64 bit), fd, path */
+		unsigned long long mask = 0;
+		int argn = getllval(tcp, &mask, 2);
+
+		return upathmatch(tcp, tcp->u_arg[argn + 1], set);
+	}
+
+	case SEN_printargs:
+		return false;
+	}
+
+	/*
+	 * Our fallback position for calls that haven't already
+	 * been handled is to just check arg[0].
+	 */
+	return upathmatch(tcp, tcp->u_arg[0], set);
 }