libbpf-tools: fix misuse of bpf_get_current_pid_tgid

bpf_get_current_pid_tgid() returns process ID in the upper 32bits,
and thread ID in lower 32 bits (both from userspace's perspective).
biosnoop and gethostlatency misuse this function.
biosnoop takes the thread ID as process ID which is not expected.
gethostlatency uses the process ID as a unique key for BPF map,
which may result in event loss or data corruption.
This commit fixes these problems.

Signed-off-by: Hengqi Chen <chenhengqi@outlook.com>

diff --git a/libbpf-tools/biosnoop.bpf.c b/libbpf-tools/biosnoop.bpf.c
index 766979672e5169682c9c1bfe0cde90fea5210588..25e1f5462e29014412171d8241a9f65bf9d2d9a9 100644 (file)
--- a/libbpf-tools/biosnoop.bpf.c
+++ b/libbpf-tools/biosnoop.bpf.c
@@ -51,7 +51,7 @@ int trace_pid(struct request *rq)
        u64 id = bpf_get_current_pid_tgid();
        struct piddata piddata = {};
 
-       piddata.pid = id;
+       piddata.pid = id >> 32;
        bpf_get_current_comm(&piddata.comm, sizeof(&piddata.comm));
        bpf_map_update_elem(&infobyreq, &rq, &piddata, 0);
        return 0;

diff --git a/libbpf-tools/gethostlatency.bpf.c b/libbpf-tools/gethostlatency.bpf.c
index 2873ad9cebd4955b2eed6c87ec77f1899e62ad01..ef26c77ee1081824d26342cfaee3056304e428ea 100644 (file)
--- a/libbpf-tools/gethostlatency.bpf.c
+++ b/libbpf-tools/gethostlatency.bpf.c
@@ -29,7 +29,9 @@ int probe_entry(struct pt_regs *ctx) {
                return 0;
 
        struct val_t val = {};
-       __u32 pid = bpf_get_current_pid_tgid() >> 32;
+       __u64 pid_tgid = bpf_get_current_pid_tgid();
+       __u32 pid = pid_tgid >> 32;
+       __u32 tid = (__u32)pid_tgid;
 
        if (targ_tgid && targ_tgid != pid)
                return 0;
@@ -39,7 +41,7 @@ int probe_entry(struct pt_regs *ctx) {
                                           (void *)PT_REGS_PARM1(ctx));
                val.pid = pid;
                val.time = bpf_ktime_get_ns();
-               bpf_map_update_elem(&start, &pid, &val, BPF_ANY);
+               bpf_map_update_elem(&start, &tid, &val, BPF_ANY);
        }
 
        return 0;
@@ -48,10 +50,12 @@ int probe_entry(struct pt_regs *ctx) {
 static __always_inline
 int probe_return(struct pt_regs *ctx) {
        struct val_t *valp;
-       __u32 pid = bpf_get_current_pid_tgid() >> 32;
+       __u64 pid_tgid = bpf_get_current_pid_tgid();
+       __u32 pid = pid_tgid >> 32;
+       __u32 tid = (__u32)pid_tgid;
        __u64 now = bpf_ktime_get_ns();
 
-       valp = bpf_map_lookup_elem(&start, &pid);
+       valp = bpf_map_lookup_elem(&start, &tid);
        if (!valp)
                return 0;
 
@@ -59,7 +63,7 @@ int probe_return(struct pt_regs *ctx) {
        valp->time = now - valp->time;
        bpf_perf_event_output(ctx, &events, BPF_F_CURRENT_CPU, valp,
                        sizeof(*valp));
-       bpf_map_delete_elem(&start, &pid);
+       bpf_map_delete_elem(&start, &tid);
        return 0;
 }
 

diff --git a/libbpf-tools/gethostlatency.c b/libbpf-tools/gethostlatency.c
index eb049a1435cdecca9da8dd7d5d3765ea7d32762f..0624acca4e02469643a1e5a844d0eaa35304488b 100644 (file)
--- a/libbpf-tools/gethostlatency.c
+++ b/libbpf-tools/gethostlatency.c
@@ -19,8 +19,8 @@
 #define PERF_POLL_TIMEOUT_MS 100
 #define warn(...) fprintf(stderr, __VA_ARGS__)
 
-volatile sig_atomic_t canceled = 0;
-pid_t traced_pid = 0;
+static volatile sig_atomic_t exiting = 0;
+static pid_t traced_pid = 0;
 
 const char *argp_program_version = "gethostlatency 0.1";
 const char *argp_program_bug_address =
@@ -35,8 +35,8 @@ const char argp_program_doc[] =
 "    gethostlatency -p 1216     # only trace PID 1216\n";
 
 static const struct argp_option opts[] = {
-       {"pid", 'p', "PID", 0, "Process ID to trace"},
-       {NULL, 'h', NULL, OPTION_HIDDEN, "Show the full help"},
+       { "pid", 'p', "PID", 0, "Process ID to trace" },
+       { NULL, 'h', NULL, OPTION_HIDDEN, "Show the full help" },
        {},
 };
 
@@ -65,7 +65,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 
 static void sig_int(int signo)
 {
-       canceled = 1;
+       exiting = 1;
 }
 
 static const char *strftime_now(char *s, size_t max, const char *format)
@@ -86,7 +86,7 @@ static const char *strftime_now(char *s, size_t max, const char *format)
        return s;
 }
 
-void handle_event(void *ctx, int cpu, void *data, __u32 data_sz)
+static void handle_event(void *ctx, int cpu, void *data, __u32 data_sz)
 {
        const struct val_t *e = data;
        char s[16] = {};
@@ -97,7 +97,7 @@ void handle_event(void *ctx, int cpu, void *data, __u32 data_sz)
                now, e->pid, e->comm, (double)e->time/1000000, e->host);
 }
 
-void handle_lost_events(void *ctx, int cpu, __u64 lost_cnt)
+static void handle_lost_events(void *ctx, int cpu, __u64 lost_cnt)
 {
        warn("lost %llu events on CPU #%d\n", lost_cnt, cpu);
 }
@@ -273,7 +273,7 @@ int main(int argc, char **argv)
        while (1) {
                if ((err = perf_buffer__poll(pb, PERF_POLL_TIMEOUT_MS)) < 0)
                        break;
-               if (canceled)
+               if (exiting)
                        goto cleanup;
        }
        warn("error polling perf buffer: %d\n", err);