From 18c8c14162d144a7a627156da7b3707d88ea895c Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Tue, 1 May 2018 20:07:28 +0200 Subject: [PATCH 1/3] Tests for detection of external pointers from context argument --- tests/python/test_clang.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/python/test_clang.py b/tests/python/test_clang.py index 6f55df27a311..e95c5cb4e0ef 100755 --- a/tests/python/test_clang.py +++ b/tests/python/test_clang.py @@ -701,6 +701,30 @@ def test_padding_types(self): self.assertEqual(ct.sizeof(table.Key), 96) self.assertEqual(ct.sizeof(table.Leaf), 16) + @skipUnless(kernel_version_ge(4,7), "requires kernel >= 4.7") + def test_probe_read_tracepoint_context(self): + text = """ +#include +TRACEPOINT_PROBE(tcp, tcp_retransmit_skb) { + struct sock *sk = (struct sock *)args->skaddr; + return sk->sk_dport; +} +""" + b = BPF(text=text) + + def test_probe_read_kprobe_ctx(self): + text = """ +#include +#include +int test(struct pt_regs *ctx) { + struct sock *sk; + sk = (struct sock *)ctx->di; + return sk->sk_dport; +} +""" + b = BPF(text=text) + fn = b.load_func("test", BPF.KPROBE) + if __name__ == "__main__": main() From b66a9c9b998b79a6343376cf442b400a77dd8ff5 Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Tue, 1 May 2018 22:10:28 +0200 Subject: [PATCH 2/3] Detect external pointers from context pointers The bcc rewriter is currently unable to detect external pointers (i.e., to a memory address that requires calls to bpf_probe_read) if they are not declared as arguments, e.g., if they are retrieved through the context argument. For example, although the two following examples translate to the same C code in the end (the bcc rewriter translates the first into the second), the sk pointer is recognized as an external pointer only in the first example. int test1(struct pt_regs *ctx, struct sock *sk) { // sk is correctly recognized as an external pointer. } int test2(struct pt_regs *ctx) { struct sock *sk = (struct sock *)ctx->di; // sk is not recognized as an external pointer. } This commit fixes that by detecting member dereferences of the context argument (i.e., the first argument of externally visible functions). It also works for the TRACEPOINT_PROBE macro. --- src/cc/frontends/clang/b_frontend_action.cc | 47 +++++++++++++++++++-- src/cc/frontends/clang/b_frontend_action.h | 3 ++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/cc/frontends/clang/b_frontend_action.cc b/src/cc/frontends/clang/b_frontend_action.cc index 15b852a30c58..9382f639c4bc 100644 --- a/src/cc/frontends/clang/b_frontend_action.cc +++ b/src/cc/frontends/clang/b_frontend_action.cc @@ -156,8 +156,9 @@ ProbeVisitor::ProbeVisitor(ASTContext &C, Rewriter &rewriter, set &m) : bool ProbeVisitor::VisitVarDecl(VarDecl *Decl) { if (Expr *E = Decl->getInit()) { - if (ProbeChecker(E, ptregs_).is_transitive()) + if (ProbeChecker(E, ptregs_).is_transitive() || IsContextMemberExpr(E)) { set_ptreg(Decl); + } } return true; } @@ -185,7 +186,7 @@ bool ProbeVisitor::VisitBinaryOperator(BinaryOperator *E) { if (ProbeChecker(E->getRHS(), ptregs_).is_transitive()) { ProbeSetter setter(&ptregs_); setter.TraverseStmt(E->getLHS()); - } else if (E->isAssignmentOp() && E->getRHS()->getStmtClass() == Stmt::CallExprClass) { + } else if (E->getRHS()->getStmtClass() == Stmt::CallExprClass) { CallExpr *Call = dyn_cast(E->getRHS()); if (MemberExpr *Memb = dyn_cast(Call->getCallee()->IgnoreImplicit())) { StringRef memb_name = Memb->getMemberDecl()->getName(); @@ -204,6 +205,9 @@ bool ProbeVisitor::VisitBinaryOperator(BinaryOperator *E) { } } } + } else if (IsContextMemberExpr(E->getRHS())) { + ProbeSetter setter(&ptregs_); + setter.TraverseStmt(E->getLHS()); } return true; } @@ -262,6 +266,40 @@ bool ProbeVisitor::VisitMemberExpr(MemberExpr *E) { return true; } +bool ProbeVisitor::IsContextMemberExpr(Expr *E) { + if (!E->getType()->isPointerType()) + return false; + + MemberExpr *Memb = dyn_cast(E->IgnoreParenCasts()); + Expr *base; + SourceLocation rhs_start, member; + bool found = false; + MemberExpr *M; + for (M = Memb; M; M = dyn_cast(M->getBase())) { + memb_visited_.insert(M); + rhs_start = M->getLocEnd(); + base = M->getBase(); + member = M->getMemberLoc(); + if (M->isArrow()) { + found = true; + break; + } + } + if (!found) { + return false; + } + if (member.isInvalid()) { + return false; + } + + if (DeclRefExpr *base_expr = dyn_cast(base->IgnoreImplicit())) { + if (base_expr->getDecl() == ctx_) { + return true; + } + } + return false; +} + SourceRange ProbeVisitor::expansionRange(SourceRange range) { return rewriter_.getSourceMgr().getExpansionRange(range); @@ -862,8 +900,11 @@ void BTypeConsumer::HandleTranslationUnit(ASTContext &Context) { if (FunctionDecl *F = dyn_cast(D)) { if (fe_.is_rewritable_ext_func(F)) { for (auto arg : F->parameters()) { - if (arg != F->getParamDecl(0) && !arg->getType()->isFundamentalType()) + if (arg == F->getParamDecl(0)) { + probe_visitor_.set_ctx(arg); + } else if (!arg->getType()->isFundamentalType()) { probe_visitor_.set_ptreg(arg); + } } probe_visitor_.TraverseDecl(D); } diff --git a/src/cc/frontends/clang/b_frontend_action.h b/src/cc/frontends/clang/b_frontend_action.h index bea5a603f58d..b091a872d9c0 100644 --- a/src/cc/frontends/clang/b_frontend_action.h +++ b/src/cc/frontends/clang/b_frontend_action.h @@ -95,7 +95,9 @@ class ProbeVisitor : public clang::RecursiveASTVisitor { bool VisitUnaryOperator(clang::UnaryOperator *E); bool VisitMemberExpr(clang::MemberExpr *E); void set_ptreg(clang::Decl *D) { ptregs_.insert(D); } + void set_ctx(clang::Decl *D) { ctx_ = D; } private: + bool IsContextMemberExpr(clang::Expr *E); clang::SourceRange expansionRange(clang::SourceRange range); template clang::DiagnosticBuilder error(clang::SourceLocation loc, const char (&fmt)[N]); @@ -106,6 +108,7 @@ class ProbeVisitor : public clang::RecursiveASTVisitor { std::set memb_visited_; std::set ptregs_; std::set &m_; + clang::Decl *ctx_; }; // A helper class to the frontend action, walks the decls From a8b4cee471062fa79c3b5b285d29a2a1040ac9d5 Mon Sep 17 00:00:00 2001 From: Paul Chaignon Date: Wed, 2 May 2018 08:05:49 +0200 Subject: [PATCH 3/3] Use older skb/kfree_skb tracepoint for tests --- tests/python/test_clang.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/python/test_clang.py b/tests/python/test_clang.py index e95c5cb4e0ef..b74025623501 100755 --- a/tests/python/test_clang.py +++ b/tests/python/test_clang.py @@ -704,10 +704,10 @@ def test_padding_types(self): @skipUnless(kernel_version_ge(4,7), "requires kernel >= 4.7") def test_probe_read_tracepoint_context(self): text = """ -#include -TRACEPOINT_PROBE(tcp, tcp_retransmit_skb) { - struct sock *sk = (struct sock *)args->skaddr; - return sk->sk_dport; +#include +TRACEPOINT_PROBE(skb, kfree_skb) { + struct sk_buff *skb = (struct sk_buff *)args->skbaddr; + return skb->protocol; } """ b = BPF(text=text)