From 0d0ddc94bb0aba5c7055dcdb239e34c3e62eaf19 Mon Sep 17 00:00:00 2001
From: Viktor Malik <viktor.malik@gmail.com>
Date: Wed, 28 Feb 2024 13:09:18 +0100
Subject: [PATCH] Fix error in dereferencing kernel double pointers

BPF verifier can detect safety of pointer accesses for BTF-based probes
(k(ret)func, iter) and therefore it is not necessary to use
bpf_probe_read_kernel inside such probes. This feature was enabled in
bpftrace by commit c2c3ab96 ("Support identifying btf type").

Unfortunately, the verifier is not able to track BTF information for
dereferences and array accesses on double pointers so, e.g. the
following script fails to load:

    # bpftrace -e 'kfunc:__module_get { print(args.module->trace_events[0]->flags);' } -v
    INFO: node count: 13
    Attaching 1 probe...

    Error log:
    reg type unsupported for arg#0 function kfunc_vmlinux___module_get#22
    0: R1=ctx(off=0,imm=0) R10=fp0
    0: (79) r1 = *(u64 *)(r1 +0)
    func '__module_get' arg0 has btf_id 250 type STRUCT 'module'
    1: R1_w=ptr_module(off=0,imm=0)
    1: (79) r1 = *(u64 *)(r1 +1128)       ; R1_w=scalar()
    2: (79) r1 = *(u64 *)(r1 +0)
    R1 invalid mem access 'scalar'
    processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0

    ERROR: Error loading program: kfunc:vmlinux:__module_get

A similar error happens when dereferencing the double pointer with `*`

    # bpftrace -e 'kfunc:__module_get { print((*args.module->trace_events)->flags);' } -v

An analogous program fails to load even when written using libbpf.

We need to use bpf_probe_read_kernel for such cases so do not propagate
the SizedType::is_btftype flag when observing a dereference or array
access of a double pointer in semantic analyser.
---
 CHANGELOG.md                         |  2 ++
 src/ast/passes/semantic_analyser.cpp | 13 +++++++++++--
 tests/runtime/regression             | 15 +++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e4509ef1f36..b9ae0c146de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -61,6 +61,8 @@ and this project adheres to
   - [#3178](https://github.com/bpftrace/bpftrace/pull/3178)
 - Field analyser: resolve fields for array accesses
   - [#3024](https://github.com/bpftrace/bpftrace/pull/3024)
+- Fix error in dereferencing kernel double pointers
+  - [#3024](https://github.com/bpftrace/bpftrace/pull/3024)
 #### Docs
 #### Tools
 
diff --git a/src/ast/passes/semantic_analyser.cpp b/src/ast/passes/semantic_analyser.cpp
index 0e005d65d4a..906acdfe8bb 100644
--- a/src/ast/passes/semantic_analyser.cpp
+++ b/src/ast/passes/semantic_analyser.cpp
@@ -1551,8 +1551,14 @@ void SemanticAnalyser::visit(ArrayAccess &arr)
   else
     arr.type = CreateNone();
   arr.type.is_internal = type.is_internal;
-  arr.type.is_btftype = type.is_btftype;
   arr.type.SetAS(type.GetAS());
+
+  // BPF verifier cannot track BTF information for double pointers so we cannot
+  // propagate is_btftype for arrays of pointers and we need to reset it on the
+  // array type as well.
+  if (arr.type.IsPtrTy())
+    type.is_btftype = false;
+  arr.type.is_btftype = type.is_btftype;
 }
 
 void SemanticAnalyser::binop_int(Binop &binop)
@@ -1875,9 +1881,12 @@ void SemanticAnalyser::visit(Unop &unop)
       unop.type = SizedType(*type.GetPointeeTy());
       if (type.IsCtxAccess())
         unop.type.MarkCtxAccess();
-      unop.type.is_btftype = type.is_btftype;
       unop.type.is_internal = type.is_internal;
       unop.type.SetAS(type.GetAS());
+
+      // BPF verifier cannot track BTF information for double pointers
+      if (!unop.type.IsPtrTy())
+        unop.type.is_btftype = type.is_btftype;
     } else if (type.IsRecordTy()) {
       // We allow dereferencing "args" with no effect (for backwards compat)
       if (type.IsCtxAccess())
diff --git a/tests/runtime/regression b/tests/runtime/regression
index 945add40da8..f51f9128049 100644
--- a/tests/runtime/regression
+++ b/tests/runtime/regression
@@ -82,3 +82,18 @@ NAME address_probe_invalid_expansion
 RUN {{BPFTRACE}} -e "uprobe:./testprogs/uprobe_test:0x$(nm ./testprogs/uprobe_test | awk '$3 == "uprobeFunction1" {print $1}') { @[probe] = count(); exit() }"
 EXPECT Attaching 1 probe...
 TIMEOUT 1
+
+# The verifier is not able to track BTF info for double pointer dereferences and
+# array accesses so we must use bpf_probe_read_kernel, otherwise the program is
+# rejected. The below two tests make sure that we handle such situations.
+NAME kfunc double pointer dereference
+PROG kfunc:__module_get { print((*args.module->trace_events)->flags); exit(); }
+AFTER lsmod
+EXPECT Attaching 1 probe...
+TIMEOUT 1
+
+NAME kfunc double pointer array access
+PROG kfunc:__module_get { print(args.module->trace_events[1]->flags); exit(); }
+AFTER lsmod
+EXPECT Attaching 1 probe...
+TIMEOUT 1