RFC: path display and path filtering features.

[Grant Edwards]

On 2011-02-18, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Thu, Feb 17, 2011 at 11:29:27AM -0600, Grant Edwards wrote:
>> I've re-written the path display/trace features.  Attached is a
>> snapshot of the diffs against the current git HEAD.  Changes since the
>> first version I posted follow:
>
> Thanks a lot.  Now we have a working PoC to discuss details.
>
>>  * Removed the code that tracked fd-table state based on syscalls.  It
>>    now does a readlink() on /proc/<pid>/fd/<fd> to find the path
>>    associated with a file descriptor.
>
> Linux kernel treats file descriptors as unsigned integers, but
> sysctl_nr_open is still limited to 0x100000, so I'd add an extra check
> for fd < 0 in getpath().

Done.

>>  * Change the -P option so that instead of accepting a colon-separated
>>    list of paths, it accepts a single path.  Multiple -P options can
>>    be specified to trace multiple paths.
>
> Maybe an attempt to exceed MAXSELECTED in pathtrace_select() should be
> treated as a fatal error.

Done.

>>  * Change the way that file descriptors are printed by the display
>>    functions.  Instead of using "%s" and a function that returns a
>>    formatted string, they now use a printfd() function analogous to
>>    the printpath() function.
>
> BTW, there is a long standing bug in decoding of file descriptors on
> 64bit architectures, and it's time to fix it in one place.
>
> For example,
> $ cat close.c
> int close(unsigned long fd);
> int main(void){return !!close(0xffffffff00000000UL);}
> $ gcc -Wall -O2 close.c -o close
> $ strace -eclose -o'|tail -1' ./close
> close(-4294967296)                      = 0
> $ strace -y -eclose -o'|tail -1' ./close
> close(-4294967296</dev/pts/1>)          = 0
>
> The fix is to change "fd" type in printfd() from long to int, and to print
> it using %d format.

Done.

>>  * Added handling (for Linux) for system calls where we need to look
>>    at something other than arg[0] for a descriptor/path.
>
> There is a lot of work to do.
> Some non-arg[0] syscalls are not listed in pathtrace_match(); for
> example, sys_dup3 and sys_old_mmap are listed but sys_dup2 and
> sys_mmap are not.

The missing dup2 appears to have been an oversight on my part.

sys_mmap is missing becuase I only examined system calls that had the
TRACE_DESC flag set in the sysent table.

I've added both of them and added the TRACE_DESC flag to the sysent
entry for sys_mmap.

> Some struct sysent records still have outdated sys_flags; for example,
> TRACE_DESC is not set for sys_mmap and sys_fadvise64*.

Fixed.

[I notice that the flags value was TF on m68k and s390x, but it was 0
for the rest.  I've changed it to TD for all architectures.]


> I'm not sure that we can ignore all cases where syscalls return file
> descriptor as their return value.

That wasn't my intent, and I don't think that's what the code does.

I meant to ignore system calls who returned file descriptors (and
therefore had the TRACE_DESC flag set), but didn't have arguments that
were file paths or open file descriptors.

I assume you're talking about are this bit of code:

  if (s->sys_func == sys_poll ||
      s->sys_func == printargs ||
      s->sys_func == sys_ppoll  ||
      s->sys_func == sys_select ||
      s->sys_func == sys_oldselect  ||
      s->sys_func == sys_pselect6  ||
      s->sys_func == sys_pipe  ||
      s->sys_func == sys_pipe2  ||
      s->sys_func == sys_eventfd2  ||
      s->sys_func == sys_eventfd  ||
      s->sys_func == sys_inotify_init1  ||
      s->sys_func == sys_timerfd_create ||
      s->sys_func == sys_timerfd_gettime ||
      s->sys_func == sys_timerfd_settime)
    {
      // these either return fd's or they do other things we don't
      // yet handle
      return 0;
    }

How's this?


  if (s->sys_func == printargs ||
      s->sys_func == sys_pipe  ||
      s->sys_func == sys_pipe2  ||
      s->sys_func == sys_eventfd2  ||
      s->sys_func == sys_eventfd  ||
      s->sys_func == sys_inotify_init1  ||
      s->sys_func == sys_timerfd_create ||
      s->sys_func == sys_timerfd_settime ||
      s->sys_func == sys_timerfd_gettime)
    {
      // these have TRACE_FILE or TRACE_DESCRIPTOR set, but they don't
      // have any file descriptor or path args to test
      return 0;
    }

  if (s->sys_func == sys_poll ||
      s->sys_func == sys_ppoll  ||
      s->sys_func == sys_select ||
      s->sys_func == sys_oldselect  ||
      s->sys_func == sys_pselect6)
    {
      // these have arguments that refer indirectly to file
      // descriptors, and we need to add code to handle them.
      return 0;
    }
    
I'm going to work on checking the fd's referred in the args to the
poll/select calls. Are there other calls in that list that have
path/fd arguments that should be tested?  


> For example,
> $ strace -s4 -y -P /lib64/libc-2.11.3.so -P /lib64/libc.so.6 /bin/echo
> open("/lib64/libc.so.6", O_RDONLY)      = 3
> read(3</lib64/libc-2.11.3.so>, "\177ELF"..., 832) = 832
> fstat(3</lib64/libc-2.11.3.so>, {st_mode=S_IFREG|0755, st_size=1465744, ...}) = 0
> close(3</lib64/libc-2.11.3.so>)         = 0
>
> The pathname passed to open(2) is a symlink, and /proc/<pid>/fd/<fd>
> points to the canonicalized pathname, so -P /lib64/libc-2.11.3.so
> won't catch this open(2) call now.

I can think of a couple options:

 1) Store both the canoical and "as-provided" versions of paths
    passed to -P.

 2) Canonicalize the pathname passed to -P, and also canonicalize
    path arguments when checking for a match. 

Either would be OK with me.
    
-- 
Grant Edwards               grant.b.edwards        Yow! Are we wet yet?
                                  at               
                              gmail.com