-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
invalid reads: type=inv expected=fp #209
Comments
for map operations. the verifier has the following constraints for safely
I think the following line // filebypid.update(&pid, (char **)&dent->d_iname); may cause trouble as pid is a pointer to an internal data structure and not a pointer to stack. On Sat, Sep 12, 2015 at 2:23 PM, Brendan Gregg notifications@github.com
|
To answer the question, what is "inv" and "fp"? On Sat, Sep 12, 2015 at 2:23 PM, Brendan Gregg notifications@github.com
|
Thanks. So pid is used successfully as a key in the "start" hash, and well as in the https://github.com/iovisor/bcc/blob/master/examples/vfsreadlat.c example. I think the problem is with (char **)&dent->d_iname. |
yes. it's the value that is causing problems. 40: (18) r1 = 0x93c8960 r1 - is a map pointer (1st argument to bpf_map_lookup call) in this case r2 = r10 - 12 which is ok On Sat, Sep 12, 2015 at 7:58 PM, Brendan Gregg notifications@github.com
|
Thanks for explanation. Need to look at verifier code to understand more.☺ On Sat, Sep 12, 2015 at 8:08 PM, 4ast notifications@github.com wrote:
|
Right. I made a mistake. u32 pid = bpf_get_current_pid_tgid(); pid is on the stack, so &pid is perfect fine as the pointer-to-key used later. if you do a map lookup, get a value returned as a pointer, and want to use returned value as the key, then you may need to copy to the stack first. On Sat, Sep 12, 2015 at 7:58 PM, Brendan Gregg notifications@github.com
|
Thanks; well, given I just did a bpf_probe_read() of name, can't I use name?
When I try this, I get the first 8 chars of the string ok, and then garbage. |
Finally have it working: #include <uapi/linux/ptrace.h>
#include <linux/blkdev.h>
#include <linux/mount.h>
BPF_HASH(start, u32);
BPF_HASH(filebypid, u32, struct dentry *);
int trace_entry(struct pt_regs *ctx, struct file *file)
{
struct dentry *dent;
u64 ts;
u32 pid = bpf_get_current_pid_tgid();
if (file != NULL && file->f_path.dentry != NULL &&
file->f_path.mnt != NULL) {
// compiles (finally), but this test doesn't exclude what I wanted
// int mnt_flags = 0;
// bpf_probe_read(&mnt_flags, sizeof(mnt_flags), &file->f_path.mnt->mnt_flags);
// if (mnt_flags & MNT_NODEV) {
// return 0;
// }
// map pid to dentry pointer
dent = file->f_path.dentry;
filebypid.update(&pid, &dent);
bpf_trace_printk("read flags %x\\n", mnt_flags);
}
ts = bpf_ktime_get_ns();
start.update(&pid, &ts);
return 0;
}
int trace_return(struct pt_regs *ctx)
{
u64 *tsp, delta;
u32 pid = bpf_get_current_pid_tgid();
tsp = start.lookup(&pid);
if (tsp != 0) {
delta = bpf_ktime_get_ns() - *tsp;
if (delta > MIN_LATENCY_NS) {
struct dentry **dentpp, *dent = 0;
dentpp = filebypid.lookup(&pid);
bpf_probe_read(&dent, sizeof(dent), dentpp);
bpf_trace_printk("slow %llx %d us %s\\n", dent, delta / 1000, dent->d_iname);
if (dent != NULL) {
filebypid.delete(&pid);
}
}
start.delete(&pid);
}
return 0;
} I'm passing over the dentry pointer, instead of the string. This took about a dozen attempts (some I still don't understand why they failed). It should get a little easier now that I have two working examples to copy from, but this was really hard to figure out. Maybe an error message that said "you need to bpf_probe_read(X)". |
FWIW, I can't match on MNT_NODEV because FSes don't respect it: # mount
/dev/xvda on / type ext4 (rw,noatime,barrier=0)
proc on /proc type proc (rw,noexec,nosuid,nodev)
sysfs on /sys type sysfs (rw,noexec,nosuid,nodev)
none on /sys/fs/cgroup type tmpfs (rw)
none on /sys/fs/fuse/connections type fusectl (rw)
none on /sys/kernel/debug type debugfs (rw)
none on /sys/kernel/security type securityfs (rw)
udev on /dev type devtmpfs (rw,mode=0755)
devpts on /dev/pts type devpts (rw,noexec,nosuid,gid=5,mode=0620)
tmpfs on /run type tmpfs (rw,noexec,nosuid,size=10%,mode=0755)
none on /run/lock type tmpfs (rw,noexec,nosuid,nodev,size=5242880)
tmpfs on /run/shm type tmpfs (rw,nosuid,nodev,size=250M)
none on /run/user type tmpfs (rw,noexec,nosuid,nodev,size=104857600,mode=0755)
none on /sys/fs/pstore type pstore (rw)
/dev/md0 on /mnt type xfs (rw,noatime)
systemd on /sys/fs/cgroup/systemd type cgroup (rw,noexec,nosuid,nodev,none,name=systemd) Many things (I think) should have nodev, but don't. Eg, /sys/kernel/debug, from which we're doing VFS operations (reading trace_pipe). |
…xpected=fp'; 'Error loading program: tracepoint:syscalls:sys_enter_write'. looks like iovisor/bcc#209 -- probe_read_str() wants its arg1 (*dst) to be a pointer to the stack, yet I'm asking it (probably) to write into PTR_TO_MAP_VALUE. https://github.com/torvalds/linux/blob/master/kernel/bpf/verifier.c
I can't figure out some errors like the following:
What actually is "type=inv expected=fp"?
Here's a test program, that in this state works:
There's a couple of blocks, that when uncommented, break it.
It's pretty noisy to run, since it prints tty vfs_read()s from ssh (hence trying to filter it based on MNT_NODEV, which I don't know yet if it works). So I'd been testing it like this:
The text was updated successfully, but these errors were encountered: