Probing syscall__renameat in Ubuntu 20.04 - can't read 4th param

[craig65535]

I created kprobes for the renameat and renameat2 syscalls:

int syscall__renameat(struct pt_regs *ctx, int oldfd, const char __user *oldname,                                                                        
                      int newfd, const char __user *newname)                                                                                             
{                                                                                                                                                        
.
.
}

int syscall__renameat2(struct pt_regs *ctx, int oldfd, const char __user *oldname,
                       int newfd, const char __user *newname, unsigned int flags)
{  
.
.
}

I can read the data at oldname fine, but newname seems to be garbage. oldfd and newfd are correct.

I had a similar problem with RHEL 8 (#2597) but that fix doesn't work here. In fact, none of the PT_REGS_PARMx point to usable strings.

I'm using kernel 5.4.0-52-generic on Ubuntu 20.04.1 LTS. BCC version is v0.16.0. LLVM tools are 8.0.1 (built from source).

Thank you

[craig65535]

Here is a test utility (copied from #2597).

Output:

actual oldfd=11, expected oldfd=11
actual oldname="AnOldName", expected="AnOldName"
actual newfd=22, expected newfd=22
actual newname="H=", expected newname="ANewName" (FAIL)
parm1="" (-89193521021096)
parm2="" (-89193521021096)
parm3="" (0)
parm4="" (0)
parm5="" (0)

Code:

#include <iostream>

#include "bcc/BPF.h"

using namespace std;

static const string _probe_renameat = R"(
#include <uapi/linux/ptrace.h>

typedef struct event {
    int64_t oldfd;
    char oldname[32];
    int64_t newfd;
    char newname[32];
    int64_t parm1_int;
    char parm1_str[32];
    int64_t parm2_int;
    char parm2_str[32];
    int64_t parm3_int;
    char parm3_str[32];
    int64_t parm4_int;
    char parm4_str[32];
    int64_t parm5_int;
    char parm5_str[32];
} event_t;

BPF_PERF_OUTPUT(events);

int syscall__renameat(struct pt_regs *ctx, int oldfd, const char __user *oldname,
                      int newfd, const char __user *newname)
{
    event_t event = {
        .oldfd = oldfd,
        .newfd = newfd,
        .parm1_int = PT_REGS_PARM1(ctx),
        .parm2_int = PT_REGS_PARM2(ctx),
        .parm3_int = PT_REGS_PARM3(ctx),
        .parm4_int = PT_REGS_PARM4(ctx),
        .parm5_int = PT_REGS_PARM5(ctx),
    };
    bpf_probe_read_str(event.oldname, sizeof(event.oldname), oldname);
    bpf_probe_read_str(event.newname, sizeof(event.newname), newname);
    bpf_probe_read_str(event.parm1_str, sizeof(event.parm1_str), (const char __user *)PT_REGS_PARM1(ctx));
    bpf_probe_read_str(event.parm2_str, sizeof(event.parm2_str), (const char __user *)PT_REGS_PARM2(ctx));
    bpf_probe_read_str(event.parm3_str, sizeof(event.parm3_str), (const char __user *)PT_REGS_PARM3(ctx));
    bpf_probe_read_str(event.parm4_str, sizeof(event.parm4_str), (const char __user *)PT_REGS_PARM4(ctx));
    bpf_probe_read_str(event.parm5_str, sizeof(event.parm5_str), (const char __user *)PT_REGS_PARM5(ctx));
    events.perf_submit(ctx, &event, sizeof(event));
    return 0;
}
)";

/* This definition must mirror the one in the BPF program above. */
typedef struct event {
    int64_t oldfd;
    char oldname[32];
    int64_t newfd;
    char newname[32];
    int64_t parm1_int;
    char parm1_str[32];
    int64_t parm2_int;
    char parm2_str[32];
    int64_t parm3_int;
    char parm3_str[32];
    int64_t parm4_int;
    char parm4_str[32];
    int64_t parm5_int;
    char parm5_str[32];
} event_t;

typedef struct test_case {
    event_t expected_event;
    bool done;
    bool success;
} test_case_t;

static void _output(void *cb_ctx, void *data, int data_size)
{
    do {
        test_case_t *test_case = (test_case_t *)cb_ctx;
        test_case->done = true;
        if (data_size < 0 || (size_t)data_size < sizeof(event_t)) {
            cerr << "data size mismatch (" << data_size << " < expected " << sizeof(event_t) << ")" << endl;
            break;
        }
        auto event = static_cast<event_t *>(data);
        bool failed = false;
        cout << "actual oldfd=" << event->oldfd << ", expected oldfd=" << test_case->expected_event.oldfd;
        if (event->oldfd != test_case->expected_event.oldfd) {
            cout << " (FAIL)";
            failed = true;
        }
        cout << endl;
        cout << "actual oldname=\"" << event->oldname << "\", expected=\"" << test_case->expected_event.oldname << "\"";
        if (strcmp(event->oldname, test_case->expected_event.oldname)) {
            cout << " (FAIL)";
            failed = true;
        }
        cout << endl;
        cout << "actual newfd=" << event->newfd << ", expected newfd=" << test_case->expected_event.newfd;
        if (event->newfd != test_case->expected_event.newfd) {
            cout << " (FAIL)";
            failed = true;
        }
        cout << endl;
        cout << "actual newname=\"" << event->newname << "\", expected newname=\"" << test_case->expected_event.newname << "\"";
        if (strcmp(event->newname, test_case->expected_event.newname)) {
            cout << " (FAIL)";
            failed = true;
        }
        cout << endl;
        cout << "parm1=\"" << event->parm1_str << "\" (" << event->parm1_int << ")" << endl;
        cout << "parm2=\"" << event->parm2_str << "\" (" << event->parm2_int << ")" << endl;
        cout << "parm3=\"" << event->parm3_str << "\" (" << event->parm3_int << ")" << endl;
        cout << "parm4=\"" << event->parm4_str << "\" (" << event->parm4_int << ")" << endl;
        cout << "parm5=\"" << event->parm5_str << "\" (" << event->parm5_int << ")" << endl;
        test_case->success = !failed;
    } while (0);
}

int main()
{
    int ret = 1;

    do {
        ebpf::BPF bpf;
        auto init_res = bpf.init(_probe_renameat);
        if (init_res.code() != 0) {
            cerr << init_res.msg() << endl;
            break;
        }

        string fnname = bpf.get_syscall_fnname("renameat");
        auto attach_res = bpf.attach_kprobe(fnname, "syscall__renameat");
        if (attach_res.code() != 0) {
            cerr << attach_res.msg() << endl;
            break;
        }

        /* Set callback */
        test_case_t test_case = {
            .expected_event = {
                11,
                "AnOldName",
                22,
                "ANewName",
                0, "", 0, "", 0, "", 0, "", 0, "",
            },
            .done = false,
            .success = false,
        };
        auto open_res = bpf.open_perf_buffer("events", &_output, nullptr, &test_case);
        if (open_res.code() != 0) {
            cerr << open_res.msg() << endl;
            break;
        }

        cout << "Started" << endl;

        /* Generate event */
        (void)renameat(test_case.expected_event.oldfd, test_case.expected_event.oldname,
                       test_case.expected_event.newfd, test_case.expected_event.newname);

        int rc;
        do {
            cout << "Polling" << endl;
            rc = bpf.poll_perf_buffer("events");
        } while (!test_case.done && rc == 0);
        if (rc < 0) {
            cerr << "Failed to poll for events" << endl;
            break;
        }

        if (!test_case.success) {
            break;
        }
        cout << "Success" << endl;
        ret = 0;
    } while (0);

    return ret;
}

[craig65535]

I just tried with BCC v0.17.0, and the behaviour is the same.

[craig65535]

Also occurs when I build with llvm-9.0.1 from the system libraries (ubuntu llvm-9-dev and libclang-9-dev packages)

[yonghong-song]

@craig65535 thanks for reporting this. This is a bcc bug. From
https://www.systutorials.com/x86-64-calling-convention-by-gcc/
the fourth argument for syscall is r10 instead cx.

If you change code like below,

     do {                                                                                                 
-        ebpf::BPF bpf;                                                                                   
+        ebpf::BPF bpf(4);                                                                                
         auto init_res = bpf.init(_probe_renameat); 

you will see actual rewriter output with the following,

int syscall__renameat(struct pt_regs *ctx)
{
#if defined(CONFIG_ARCH_HAS_SYSCALL_WRAPPER) && !defined(__s390x__)
 struct pt_regs * __ctx = ctx->di;
 int oldfd; bpf_probe_read(&oldfd, sizeof(oldfd), &__ctx->di);
 const char __user *oldname; bpf_probe_read(&oldname, sizeof(oldname), &__ctx->si);
 int newfd; bpf_probe_read(&newfd, sizeof(newfd), &__ctx->dx);
 const char __user *newname; bpf_probe_read(&newname, sizeof(newname), &__ctx->cx);
#else
 int oldfd = ctx->di; const char __user *oldname = ctx->si; int newfd = ctx->dx; const char __user *newnam
e = ctx->cx;
#endif

    event_t event = {

In the above, to get newname, __ctx->cx is used and that is incorrect.

The following is a source workaround,

diff --git a/rename.c b/rename.c
index 25d131c..24e04b9 100644
--- a/rename.c
+++ b/rename.c
@@ -29,6 +29,9 @@ BPF_PERF_OUTPUT(events);
 int syscall__renameat(struct pt_regs *ctx, int oldfd, const char __user *oldname,
                       int newfd, const char __user *newname)
 {
+    struct pt_regs *ctx2 = (struct pt_regs *)ctx->di;
+    bpf_probe_read(&newname, sizeof(newname), &ctx2->r10);
+
     event_t event = {
         .oldfd = oldfd,
         .newfd = newfd,

And I got

[[email protected] ~/work/test/bcc/3150]$ sudo ./a.out                                                   
Started                                                                                                   
Polling                                                                                                   
actual oldfd=11, expected oldfd=11                                                                        
actual oldname="AnOldName", expected="AnOldName"
actual newfd=22, expected newfd=22
actual newname="ANewName", expected newname="ANewName"
parm1="" (-60473126142120) 
parm2="" (0)
parm3="" (65068776)
parm4="" (1604246507)
parm5="ANewName" (140720352935952)
Success

Note that your debug using PT_REGS_PARM1() is not correct as one level indirection is needed like the above rewritten code.

Will fix the bcc soon.

[craig65535]

Thank you for the comprehensive response! I'll patch my code and wait for a proper fix in bcc.