[PATCH v3 4/6] Improve fd filtering
Eugene Syromiatnikov
esyr at redhat.com
Mon Jul 3 12:03:00 UTC 2017
On Thu, Jun 29, 2017 at 02:46:13PM +0700, Nikolay Marchuk wrote:
> * basic_actions (apply_read, apply_write): Add syscall check.
> * basic_filters (run_fd_filter): Implement more accurate fd filtering.
> * defs.h (read_dumped, write_dumped): Add new flag checking macroses.
> * syscall.c (dumpio): And use them.
> ---
> basic_actions.c | 30 ++++++++++-
> basic_filters.c | 163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> defs.h | 2 +
> syscall.c | 4 +-
> 4 files changed, 191 insertions(+), 8 deletions(-)
>
> diff --git a/basic_actions.c b/basic_actions.c
> index 1eb597e..bd05832 100644
> --- a/basic_actions.c
> +++ b/basic_actions.c
> @@ -26,6 +26,7 @@
> */
> #include "defs.h"
> #include "filter.h"
> +#include "syscall.h"
>
> bool
> is_traced(struct tcb *tcp)
> @@ -131,13 +132,38 @@ void free_inject(void *_priv_data)
> void
> apply_read(struct tcb *tcp, void *_priv_data)
> {
> - tcp->qual_flg |= QUAL_READ;
> + switch (tcp->s_ent->sen) {
> + case SEN_read:
> + case SEN_pread:
> + case SEN_recv:
> + case SEN_recvfrom:
> + case SEN_mq_timedreceive:
> + case SEN_readv:
> + case SEN_preadv:
> + case SEN_preadv2:
> + case SEN_recvmsg:
> + case SEN_recvmmsg:
> + tcp->qual_flg |= QUAL_READ;
> + }
Not sure whether this change is needed, since dumpio() performs the scno
check anyway.
> }
>
> void
> apply_write(struct tcb *tcp, void *_priv_data)
> {
> - tcp->qual_flg |= QUAL_WRITE;
> + switch (tcp->s_ent->sen) {
> + case SEN_write:
> + case SEN_pwrite:
> + case SEN_send:
> + case SEN_sendto:
> + case SEN_mq_timedsend:
> + case SEN_writev:
> + case SEN_pwritev:
> + case SEN_pwritev2:
> + case SEN_vmsplice:
> + case SEN_sendmsg:
> + case SEN_sendmmsg:
> + tcp->qual_flg |= QUAL_WRITE;
> + }
Ditto here.
> }
>
> void
> diff --git a/basic_filters.c b/basic_filters.c
> index d0816a4..82f1656 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -27,6 +27,8 @@
> */
> #include "defs.h"
> #include <regex.h>
> +#include <poll.h>
> +#include "syscall.h"
>
> typedef unsigned int number_slot_t;
> #define BITS_PER_SLOT (sizeof(number_slot_t) * 8)
> @@ -412,14 +414,167 @@ parse_fd_filter(const char *str, const char *const name)
> return set;
> }
>
> +static bool
> +is_fd_in_set(int fd, struct number_set *set) {
> + if (fd < 0)
> + return false;
> + 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 = (struct number_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->u_arg[0], set);
> + }
> + if (!(s_ent->sys_flags & (TRACE_DESC | TRACE_NETWORK)))
> + return false;
> + switch (s_ent->sen) {
> + case SEN_dup2:
> + case SEN_dup3:
> + case SEN_kexec_file_load:
> + case SEN_sendfile:
> + case SEN_sendfile64:
> + case SEN_tee:
> + return is_fd_in_set(tcp->u_arg[0], set)
> + || is_fd_in_set(tcp->u_arg[1], set);
> + case SEN_linkat:
> + case SEN_renameat2:
> + case SEN_renameat:
> + case SEN_copy_file_range:
> + case SEN_splice:
> + return is_fd_in_set(tcp->u_arg[0], set)
> + || is_fd_in_set(tcp->u_arg[2], set);
> +
> + case SEN_old_mmap:
> +#if defined(S390)
> + case SEN_old_mmap_pgoff:
> +#endif
> + case SEN_mmap:
> + case SEN_mmap_4koff:
> + case SEN_mmap_pgoff:
> + case SEN_ARCH_mmap:
> + return is_fd_in_set(tcp->u_arg[4], set);
> + case SEN_symlinkat:
> + return is_fd_in_set(tcp->u_arg[1], set);
> + case SEN_epoll_ctl:
> + return is_fd_in_set(tcp->u_arg[2], set);
> + case SEN_fanotify_mark:
> + return is_fd_in_set(tcp->u_arg[3], set);
> + case SEN_oldselect:
> + case SEN_pselect6:
> + case SEN_select:
> + {
> + int i, j;
> + int nfds;
> + kernel_ulong_t *args;
> + kernel_ulong_t select_args[5];
> + unsigned int oldselect_args[5];
> + unsigned int fdsize;
> + fd_set *fds;
> +
> + if (SEN_oldselect == s_ent->sen) {
> + if (sizeof(*select_args) == sizeof(*oldselect_args)) {
> + if (umove(tcp, tcp->u_arg[0], &select_args)) {
> + return false;
> + }
> + } else {
> + unsigned int n;
> +
> + if (umove(tcp, tcp->u_arg[0], &oldselect_args)) {
> + return false;
> + }
> +
> + for (n = 0; n < 5; ++n) {
> + select_args[n] = oldselect_args[n];
> + }
> + }
> + args = select_args;
> + } else {
> + args = tcp->u_arg;
> + }
> +
> + /* Kernel truncates arg[0] to int, we do the same. */
> + nfds = (int) args[0];
> + /* Kernel rejects negative nfds, so we don't parse it either. */
> + if (nfds <= 0)
> + return false;
> + /* Beware of select(2^31-1, NULL, NULL, NULL) and similar... */
> + if (nfds > 1024*1024)
> + nfds = 1024*1024;
> + fdsize = (((nfds + 7) / 8) + current_wordsize-1) & -current_wordsize;
> + fds = xmalloc(fdsize);
> +
> + for (i = 1; i <= 3; ++i) {
> + if (args[i] == 0)
> + continue;
> + if (umoven(tcp, args[i], fdsize, fds) < 0) {
> + continue;
> + }
> + for (j = 0;; j++) {
> + j = next_set_bit(fds, j, nfds);
> + if (j < 0)
> + break;
> + if (is_fd_in_set(j, set)) {
> + free(fds);
> + return true;
> + }
> + }
> + }
> + free(fds);
> + return false;
> + }
> + case SEN_poll:
> + case SEN_ppoll:
> + {
> + struct pollfd fds;
> + unsigned nfds;
> + kernel_ulong_t start, cur, end;
> +
> + start = tcp->u_arg[0];
> + nfds = tcp->u_arg[1];
> +
> + end = start + sizeof(fds) * nfds;
> +
> + if (nfds == 0 || end < start)
> + return false;
> +
> + for (cur = start; cur < end; cur += sizeof(fds))
> + if ((umove(tcp, cur, &fds) == 0)
> + && is_fd_in_set(fds.fd, set))
> + return true;
> +
> + return false;
> + }
> + 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->u_arg[0], set);
> }
ince this code is basically the same as pathtrace_match(), isn't it possible
to refactor this check into a common function which would be called by both
pathtrace_match and run_fd_filter?
>
> void
> diff --git a/defs.h b/defs.h
> index 21dec65..4b3516b 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -283,6 +283,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 read_dumped(tcp) ((tcp)->qual_flg & QUAL_READ)
> +#define write_dumped(tcp) ((tcp)->qual_flg & QUAL_WRITE)
To me, something like dump_read() or dumping_read() sounds more natural
and aligned with the existing qualification checks.
> #define filtered(tcp) ((tcp)->flags & TCB_FILTERED)
> #define hide_log(tcp) ((tcp)->flags & TCB_HIDE_LOG)
>
> diff --git a/syscall.c b/syscall.c
> index b6e1ba3..c782f99 100644
> --- a/syscall.c
> +++ b/syscall.c
> @@ -465,7 +465,7 @@ dumpio(struct tcb *tcp)
> if (fd < 0)
> return;
>
> - if (tcp->qual_flg & QUAL_READ) {
> + if (read_dumped(tcp)) {
Why change this place two times, first time in the patch "Introduce new
filtering architecture", and not just introduce those *_dumped macros
there?
> switch (tcp->s_ent->sen) {
> case SEN_read:
> case SEN_pread:
> @@ -488,7 +488,7 @@ dumpio(struct tcb *tcp)
> return;
> }
> }
> - if (tcp->qual_flg & QUAL_WRITE) {
> + if (write_dumped(tcp)) {
Ditto here.
> switch (tcp->s_ent->sen) {
> case SEN_write:
> case SEN_pwrite:
> --
> 2.1.4
More information about the Strace-devel
mailing list