On Tue, Apr 5, 2022 at 2:46 PM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
> uprobe/uretprobe tests don't do any validation of arguments/return values,
> and without this we can't be sure we are attached to the right function,
> or that we are indeed attached to a uprobe or uretprobe. To fix this
> validate argument and return value for auto-attached functions.
>
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/uprobe_autoattach.c | 16 ++++++++++----
> .../selftests/bpf/progs/test_uprobe_autoattach.c | 25 +++++++++++++++-------
> 2 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> index 03b15d6..ff85f1f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> @@ -5,14 +5,16 @@
> #include "test_uprobe_autoattach.skel.h"
>
> /* uprobe attach point */
> -static void autoattach_trigger_func(void)
> +static noinline int autoattach_trigger_func(int arg)
> {
> - asm volatile ("");
don't remove asm volatile, with static functions compiler can still do
function transformations and stuff. From GCC documentation for
noinline attribute ([0]):
noinline
This function attribute prevents a function from being
considered for inlining. If the function does not have side effects,
there are optimizations other than inlining that cause function calls
to be optimized away, although the function call is live. To keep such
calls from being optimized away, put
asm ("");
[0] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
So I suppose noinline + asm volatile is the most safe combination :)
for static functions
> + return arg + 1;
> }
>
> void test_uprobe_autoattach(void)
> {
> struct test_uprobe_autoattach *skel;
> + int trigger_val = 100;
> + size_t malloc_sz = 1;
> char *mem;
>
> skel = test_uprobe_autoattach__open_and_load();
[...]
>
> -SEC("uretprobe/libc.so.6:free")
> +SEC("uretprobe/libc.so.6:getpid")
> int handle_uretprobe_byname2(struct pt_regs *ctx)
> {
> - uretprobe_byname2_res = 4;
> + if (PT_REGS_RC_CORE(ctx) == uretprobe_byname2_rc)
> + uretprobe_byname2_res = 4;
Instead of checking equality on BPF side, it's more convenient to
record the actual captured value into global variable and let
user-space part check and log it properly. So if something goes wrong,
we don't just have matched/not matched flag, but we see an actual
value captured.
With that, let's better switch uretprobe from free to malloc (we
additionally check that uprobe and uretprobe can coexist properly on
the same function) and capture returned pointer from malloc(). We can
then compare that pointer to the mem variable. How cool is that? :)
> return 0;
> }
>
> --
> 1.8.3.1
>
