Skip to content

Commit

Permalink
libbpf-tools: fix misuse of bpf_get_current_pid_tgid
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
chenhengqi authored and yonghong-song committed May 25, 2021
1 parent 528be5e commit b1ebd4f
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 14 deletions.
2 changes: 1 addition & 1 deletion libbpf-tools/biosnoop.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 9 additions & 5 deletions libbpf-tools/gethostlatency.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -48,18 +50,20 @@ 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;

// update time from timestamp to delta
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;
}

Expand Down
16 changes: 8 additions & 8 deletions libbpf-tools/gethostlatency.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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" },
{},
};

Expand Down Expand Up @@ -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)
Expand All @@ -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] = {};
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b1ebd4f

Please sign in to comment.