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 ------------------------------------------------------------------------------ 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@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel