Skip to content

Commit

Permalink
[kfunc/kretfunc] fix arg and retval resolution
Browse files Browse the repository at this point in the history
In the case of functions that have arguments that are larger
than the size of the registers (e.g. '__sys_bpf' whose second
arg, 'bpfptr_t uattr', which has a size of 16 bytes) the
'retval' for kretfunc/fexit is not correct.

What's needed is to resolve the btf type size and use that
(by dividing by the register size) to determine the index into
the args array.
  • Loading branch information
Jordan Rome authored and viktormalik committed Dec 15, 2023
1 parent 1772c21 commit d179793
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 4 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ and this project adheres to
- [#2851](https://github.com/iovisor/bpftrace/pull/2851)
- Fix symbolication on for 32-bit userspcae and 64-bit kernel
- [#2869](https://github.com/iovisor/bpftrace/pull/2869)
- Fix retval for kretfunc/fexit
- [#2864](https://github.com/iovisor/bpftrace/pull/2864)
#### Docs
#### Tools
- Add PPID field to `execsnoop.bt`
Expand Down
13 changes: 9 additions & 4 deletions src/btf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "tracefs.h"
#include "types.h"
#include "utils.h"
#include <cmath>
#include <cstring>
#include <fcntl.h>
#include <iostream>
Expand Down Expand Up @@ -450,30 +451,34 @@ Struct BTF::resolve_args(const std::string &func, bool ret)

Struct args(0, false);
int j = 0;
int arg_idx = 0;
__u32 type_size = 0;
for (; j < vlen; j++, p++)
{
const char *str = btf_str(func_id.btf, p->name_off);
if (!str)
throw std::runtime_error("failed to resolve arguments");

SizedType stype = get_stype(BTFId{ .btf = func_id.btf, .id = p->type });
stype.funcarg_idx = j;
stype.funcarg_idx = arg_idx;
stype.is_funcarg = true;
args.AddField(str, stype, args.size, std::nullopt, false);
// kfunc (fentry/fexit) args are stored in a u64 array.
// Note that it's ok to represent them by a struct as we will use GEP with
// funcarg_idx to access them in codegen.
args.size += 8;
type_size = btf__resolve_size(func_id.btf, p->type);
args.size += type_size;
arg_idx += std::ceil((float)type_size / (float)8);
}

if (ret)
{
SizedType stype = get_stype(BTFId{ .btf = func_id.btf, .id = t->type });
stype.funcarg_idx = j;
stype.funcarg_idx = arg_idx;
stype.is_funcarg = true;
args.AddField("$retval", stype, args.size, std::nullopt, false);
// kfunc (fentry/fexit) args (incl. retval) are stored in a u64 array
args.size += 8;
args.size += btf__resolve_size(func_id.btf, t->type);
}
return args;
}
Expand Down
8 changes: 8 additions & 0 deletions tests/runtime/probe
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ REQUIRES_FEATURE get_func_ip
TIMEOUT 5
AFTER ./testprogs/syscall read

NAME kretfunc large args
PROG kretfunc:__sys_bpf { if (args.cmd == 1111 && args.size == 2222 && (int64)retval == -7) { printf("SUCCESS %d\n", pid); exit(); }}
EXPECT SUCCESS [0-9][0-9]*
REQUIRES_FEATURE kfunc
REQUIRES_FEATURE btf
TIMEOUT 5
AFTER ./testprogs/kfunc_args 1111 2222

# Sanity check for fentry/fexit alias
NAME fentry
PROG fentry:vfs_read { printf("SUCCESS %d\n", pid); exit(); }
Expand Down
20 changes: 20 additions & 0 deletions tests/testprogs/kfunc_args.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#include <linux/bpf.h>
#include <linux/unistd.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char *argv[])
{
if (argc != 3)
{
return 1;
}
usleep(1000000);

union bpf_attr attr;
int cmd = atoi(argv[1]);
unsigned int size = atoi(argv[2]);
syscall(__NR_bpf, cmd, &attr, size);

return 0;
}

0 comments on commit d179793

Please sign in to comment.