Fix external pointer propagation in nested dereferences (#1837)

* Fix external pointer propagation in nested dereferences

and fix the count of indirections for addrof of member dereferences
(&A->b).

In nested dereferences, a dereference of an external pointer may give
a new external pointer.  For example, if A is an external pointer,
then *A and A->b should also be considered as external pointers when
appropriate (e.g., in **A or *(A->b)).

In addition, a member dereference is a dereference, so we need to
count it when counting the number of indirections in ProbeChecker.
If we don't, *(&A->b) won't be rewritten correctly as &A->b will be
considered a pointer to an external pointer.

* Tests for the count of indirections in nested dereferences

src/cc/frontends/clang/b_frontend_action.cc

@@ -136,13 +136,37 @@ class ProbeChecker : public RecursiveASTVisitor<ProbeChecker> {
       needs_probe_ = true;
       return false;
     }
+    if (M->isArrow()) {
+      /* In A->b, if A is an external pointer, then A->b should be considered
+       * one too.  However, if we're taking the address of A->b
+       * (nb_derefs_ < 0), we should take it into account for the number of
+       * indirections; &A->b is a pointer to A with an offset. */
+      if (nb_derefs_ >= 0) {
+        ProbeChecker checker = ProbeChecker(M->getBase(), ptregs_,
+                                            track_helpers_, is_assign_);
+        if (checker.needs_probe() && checker.get_nb_derefs() == 0) {
+          needs_probe_ = true;
+          return false;
+        }
+      }
+      nb_derefs_++;
+    }
     return true;
   }
   bool VisitUnaryOperator(UnaryOperator *E) {
-    if (E->getOpcode() == UO_Deref)
+    if (E->getOpcode() == UO_Deref) {
+      /* In *A, if A is an external pointer, then *A should be considered one
+       * too. */
+      ProbeChecker checker = ProbeChecker(E->getSubExpr(), ptregs_,
+                                          track_helpers_, is_assign_);
+      if (checker.needs_probe() && checker.get_nb_derefs() == 0) {
+        needs_probe_ = true;
+        return false;
+      }
       nb_derefs_++;
-    else if (E->getOpcode() == UO_AddrOf)
+    } else if (E->getOpcode() == UO_AddrOf) {
       nb_derefs_--;
+    }
     return true;
   }
   bool VisitDeclRefExpr(DeclRefExpr *E) {
@@ -406,13 +430,6 @@ bool ProbeVisitor::VisitUnaryOperator(UnaryOperator *E) {
   rewriter_.InsertTextAfterToken(expansionLoc(sub->getLocEnd()), post);
   return true;
 }
-bool ProbeVisitor::VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
-  // &({..._val; bpf_probe_read(&_val, ...); _val;})[0] is permitted
-  // by C standard.
-  if (is_addrof_)
-    is_addrof_ = false;
-  return true;
-}
 bool ProbeVisitor::VisitMemberExpr(MemberExpr *E) {
   if (memb_visited_.find(E) != memb_visited_.end()) return true;
 

src/cc/frontends/clang/b_frontend_action.h

@@ -102,7 +102,6 @@ class ProbeVisitor : public clang::RecursiveASTVisitor<ProbeVisitor> {
   bool VisitBinaryOperator(clang::BinaryOperator *E);
   bool VisitUnaryOperator(clang::UnaryOperator *E);
   bool VisitMemberExpr(clang::MemberExpr *E);
-  bool VisitArraySubscriptExpr(clang::ArraySubscriptExpr *E);
   void set_ptreg(std::tuple<clang::Decl *, int> &pt) { ptregs_.insert(pt); }
   void set_ctx(clang::Decl *D) { ctx_ = D; }
   std::set<std::tuple<clang::Decl *, int>> get_ptregs() { return ptregs_; }

tests/python/test_clang.py

@@ -429,7 +429,7 @@ int test(struct pt_regs *ctx, struct sk_buff *skb) {
 }""")
         b.load_func("test", BPF.KPROBE)
 
-    def test_probe_member_expr(self):
+    def test_probe_member_expr_deref(self):
         b = BPF(text="""
 #include <uapi/linux/ptrace.h>
 #include <linux/netdevice.h>
@@ -442,6 +442,19 @@ int test(struct pt_regs *ctx, struct sk_buff *skb) {
 }""")
         b.load_func("test", BPF.KPROBE)
 
+    def test_probe_member_expr(self):
+        b = BPF(text="""
+#include <uapi/linux/ptrace.h>
+#include <linux/netdevice.h>
+struct leaf { struct sk_buff *ptr; };
+int test(struct pt_regs *ctx, struct sk_buff *skb) {
+    struct leaf l = {};
+    struct leaf *lp = &l;
+    lp->ptr = skb;
+    return l.ptr->priority;
+}""")
+        b.load_func("test", BPF.KPROBE)
+
     def test_unop_probe_read(self):
         text = """
 #include <linux/blkdev.h>
@@ -498,6 +511,44 @@ int test(struct pt_regs *ctx, struct sock *sk) {
     *ptr3 = ptr2;
     return subtest(ptr3);
 }
+"""
+        b = BPF(text=text)
+        fn = b.load_func("test", BPF.KPROBE)
+
+    def test_probe_read_nested_member1(self):
+        text = """
+#include <net/inet_sock.h>
+int test(struct pt_regs *ctx, struct sock *skp) {
+    u32 *daddr = &skp->sk_daddr;
+    return *daddr;
+}
+"""
+        b = BPF(text=text)
+        fn = b.load_func("test", BPF.KPROBE)
+
+    def test_probe_read_nested_member2(self):
+        text = """
+#include <uapi/linux/ptrace.h>
+struct sock {
+    u32 **sk_daddr;
+};
+int test(struct pt_regs *ctx, struct sock *skp) {
+    u32 *daddr = *(skp->sk_daddr);
+    return *daddr;
+}
+"""
+        b = BPF(text=text)
+        fn = b.load_func("test", BPF.KPROBE)
+
+    def test_probe_read_nested_member3(self):
+        text = """
+#include <uapi/linux/ptrace.h>
+struct sock {
+    u32 *sk_daddr;
+};
+int test(struct pt_regs *ctx, struct sock *skp) {
+    return *(&skp->sk_daddr);
+}
 """
         b = BPF(text=text)
         fn = b.load_func("test", BPF.KPROBE)