On 2011-02-18, Dmitry V. Levin <[email protected]> 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
------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel
