Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add helper: convert FD to pathname #2544

Closed
wants to merge 1 commit into from

Conversation

ethercflow
Copy link
Contributor

Fixes: #237

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song
Copy link
Collaborator

[buildbot, ok to test]

struct task_struct *curr = (struct task_struct *)bpf_get_current_task();

bpf_probe_read(&files, sizeof(files), &curr->files);
bpf_probe_read(&fdt, sizeof(fdt), &files->fdt);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If bpf_probe_read() has issues (e.g., page fault) to read the contents, it will zero'ing the buffer and returns a negative value. With address of zero, all subsequent bpf_probe_read() will return -EFAULT, so eventually, dn.name will 0. I guess it is okay not to check bpf_probe_read() return value since we optimize for common case where bpf_probe_read() unlikely to fail. But could you add comments about why this mechanism works.

Copy link
Collaborator

@yonghong-song yonghong-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a test case to use this helper?

@brendangregg
Copy link
Member

This doesn't provide fd2path -- it's providing fd2filename. I'll update the original ticket, as we've discussed this several times since then.

It may be ok to merge this PR after changing it to fd2filename. In addition, we'd really like an fd2path -- I'd use it more often than fd2filename. I'll follow up in #237 with more detail.

@ethercflow
Copy link
Contributor Author

could you add a test case to use this helper?

Thanks, I'll add it later

@ethercflow
Copy link
Contributor Author

This doesn't provide fd2path -- it's providing fd2filename. I'll update the original ticket, as we've discussed this several times since then.

It may be ok to merge this PR after changing it to fd2filename. In addition, we'd really like an fd2path -- I'd use it more often than fd2filename. I'll follow up in #237 with more detail.

Thanks, I'll change this, and do some work with real fd2path.

@yonghong-song
Copy link
Collaborator

fd2path is indeed more helpful. I have seen people using bpf_probe_read to traverse 2 or 3 level of path hierarchy to get an idea of the path signature. A helper returning the whole path to a buffer will be more efficient and complete than such a hack.

@yonghong-song
Copy link
Collaborator

Your newly included header files

# include <linux/sched.h>
# include <linux/fdtable.h>

caused test failures as new headers bring in new definitions conflicting some tests existing function/type definitions.

@ethercflow
Copy link
Contributor Author

fd2path is indeed more helpful. I have seen people using bpf_probe_read to traverse 2 or 3 level of path hierarchy to get an idea of the path signature. A helper returning the whole path to a buffer will be more efficient and complete than such a hack.

Get it, I'll do some research like https://www.kernel.org/doc/html/latest/filesystems/path-lookup.html#introduction-to-pathname-lookup then get a way to implement this.

@ethercflow
Copy link
Contributor Author

@yonghong-song @brendangregg

Update: I've implemented a real fd2path by add a new BPF_CALL in bpf-next (local testing, hasn't committed yet). The output also support pipe and socket. The output looks as below:

 systemd-journal-200   [000] ....  1666.350020: 0:  fd: 18 path: /proc/1/cgroup
         systemd-1     [000] ....  1666.351304: 0:  fd: 42 path: /sys/fs/cgroup/systemd/system.slice/system-serial\134x2dgetty.s
         systemd-1     [000] ....  1666.352376: 0:  fd: 42 path: /proc/807/stat
         systemd-1     [000] ....  1666.352605: 0:  fd: 42 path: /sys/devices/virtual/tty/console/active
        (agetty)-807   [000] ....  1666.353314: 0:  fd: 20 path: /proc/807/fd
 systemd-journal-200   [000] ....  1666.353961: 0:  fd: 18 path: /proc/1/cgroup
 systemd-journal-200   [000] ....  1666.354346: 0:  fd: 18 path: /proc/1/comm
          agetty-807   [000] ....  1666.355144: 0:  fd: 3 path: socket:[15605]
          agetty-807   [000] ....  1666.355173: 0:  fd: 1 path: socket:[15605]
              ls-816   [000] ....  1669.418335: 0:  fd: 1 path: pipe:[15643]
bash-812   [000] ....  1669.419105: 0:  fd: 3 path: /etc/bash_completion.d/git-prompt
            bash-812   [000] ....  1669.419387: 0:  fd: 3 path: /usr/lib/git-core/git-sh-prompt
            bash-812   [000] ....  1669.421457: 0:  fd: 3 path: /root/.profile
            bash-812   [000] ....  1669.421754: 0:  fd: 3 path: /root/.bashrc
            mesg-817   [000] ....  1669.422668: 0:  fd: 3 path: /etc/ld.so.cache
            mesg-817   [000] ....  1669.422917: 0:  fd: 3 path: /lib/x86_64-linux-gnu/libc-2.24.so
            mesg-817   [000] ....  1669.423310: 0:  fd: 2 path: /dev/ttyS0
            mesg-817   [000] ....  1669.423627: 0:  fd: 3 path: /dev/ttyS0

With the test code:

#!/usr/bin/python

from __future__ import print_function
from bcc import BPF

bpf_text="""
#include <uapi/linux/ptrace.h>

TRACEPOINT_PROBE(syscalls, sys_enter_newfstat) {
    char path[128] = {'0'};
    bpf_fd2path(path, 128, args->fd);
    bpf_trace_printk(" fd: %d path: %s\\n", args->fd, path);
    return 0;
}
"""

BPF(text=bpf_text).trace_print()

I will commit this as soon as possible.

@ethercflow
Copy link
Contributor Author

ethercflow commented Oct 17, 2019

I have committed to http://patchwork.ozlabs.org/patch/1179287/ PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FD to pathname
3 participants