Commit 66d28635 authored by yonghong-song's avatar yonghong-song Committed by GitHub

prevent bpf_probe_read MemberExpr rewrite if not rewritable (#1827)

For the test case in this patch below,
 #define _(P) ({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;})
 int count_tcp(struct pt_regs *ctx, struct sk_buff *skb) {
     return _(TCP_SKB_CB(skb)->tcp_gso_size);
 }

The clang AST will consider the whole `_(TCP_SKB_CB(skb)->tcp_gso_size)`
as a MemberExpr during AST traversal. However, it will consider
the start location of the member expression not rewritable.
Without this patch, we will get an error like below:
    /virtual/main.c:15:44: error: expected ';' after return statement
    return _(TCP_SKB_CB(skb)->tcp_gso_size)); _val; });
Basically, the start of bpf_probe_read() rewritingg failed but
later part succeeded, so the code becomes uncompilable.

Previously, we did not see such issues, but as rewriter got
more smarter this bug is exposed.

This patch fixed the issue by preventing rewriting the whole
expression if the start location for the member expression is
not rewritable.
Signed-off-by: default avatarYonghong Song <yhs@fb.com>
parent e07f2ed3
...@@ -419,6 +419,9 @@ bool ProbeVisitor::VisitMemberExpr(MemberExpr *E) { ...@@ -419,6 +419,9 @@ bool ProbeVisitor::VisitMemberExpr(MemberExpr *E) {
return false; return false;
} }
if (!rewriter_.isRewritable(E->getLocStart()))
return false;
/* If the base of the dereference is a call to another function, we need to /* If the base of the dereference is a call to another function, we need to
* visit that function first to know if a rewrite is necessary (i.e., if the * visit that function first to know if a rewrite is necessary (i.e., if the
* function returns an external pointer). */ * function returns an external pointer). */
......
...@@ -76,6 +76,18 @@ int count_foo(struct pt_regs *ctx, unsigned long a, unsigned long b) { ...@@ -76,6 +76,18 @@ int count_foo(struct pt_regs *ctx, unsigned long a, unsigned long b) {
b = BPF(text=text, debug=0) b = BPF(text=text, debug=0)
fn = b.load_func("count_foo", BPF.KPROBE) fn = b.load_func("count_foo", BPF.KPROBE)
def test_probe_read3(self):
text = """
#define KBUILD_MODNAME "foo"
#include <net/tcp.h>
#define _(P) ({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;})
int count_tcp(struct pt_regs *ctx, struct sk_buff *skb) {
return _(TCP_SKB_CB(skb)->tcp_gso_size);
}
"""
b = BPF(text=text)
fn = b.load_func("count_tcp", BPF.KPROBE)
def test_probe_read_whitelist1(self): def test_probe_read_whitelist1(self):
text = """ text = """
#define KBUILD_MODNAME "foo" #define KBUILD_MODNAME "foo"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment