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

Reply via email to