iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.63k stars 3.89k forks source link

Leaf() considered the padded space as a valid struct member #2255

Open lcp opened 5 years ago

lcp commented 5 years ago

I found a regression since 0.6.0. If there is a struct member is aligned, Leaf() may consider the padded space as a valid member.

Here is the bug reproducer for x86_64:

#!/usr/bin/python 

from bcc import BPF
from ctypes import c_int

prog="""
struct test {
    int a;
    u64 b;
};

BPF_HASH(my_hash, u64, struct test, 4096);

int
kprobe__sys_clone(void *ctx)
{
    u64 idx = 0;
    struct test *leaf = my_hash.lookup(&idx);
    if (leaf) {
        bpf_trace_printk("my_hash[0].a = %d\\n", leaf->a);
        bpf_trace_printk("my_hash[0].b = %d\\n", leaf->b);
    }
    return 0;
}
"""

b=BPF(text=prog)
my_hash = b.get_table("my_hash")
my_hash[c_int(0)] = my_hash.Leaf(1, 2)

try:
    b.trace_print()
except KeyboardInterrupt:
    exit()

The code looked legit but it failed with the following error.

Traceback (most recent call last):
  File "./phantom-member.py", line 29, in <module>
    my_hash[c_int(0)] = my_hash.Leaf(1, 2)
TypeError: expected bytes, int found

The program worked again when I added a bytes parameter to Leaf() like this:

my_hash[c_int(0)] = my_hash.Leaf(1, b"", 2)

or added __attribute__((packed)) to struct test, so it's obvious an issue about padding.

Since the issue happened between 0.5.0 and 0.6.0, it's likely related to https://github.com/iovisor/bcc/commit/538a84e1f821a97468a55d0e97a5c0a4617a1271 and https://github.com/iovisor/bcc/commit/b32b4a5fffb61ae6fd5e75e30c06763ade8ed1a4.

yonghong-song commented 5 years ago

The issue exists in the trunk as well. The reason, as you described, is Leaf data structure with extra padding member and the constructor value sequence (1, 2) does not match structure layout any more. Need to think how to resolve this automatically.