From 026a62bfcd2189273d1fe9f3e2af47802dd3f577 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Mon, 8 Mar 2021 17:31:26 +0100 Subject: [PATCH 1/2] Zero on stack data before passing it to bpf_perf_event_output When starting unixdump, BPF verifier complains about invalid read from stack: 177: (85) call bpf_perf_event_output#25 invalid indirect read from stack off -208+35 size 208 This is because struct notify_t object that we allocate on stack might not be initialized. See similar issue description at: https://github.com/iovisor/bcc/issues/2623#issuecomment-560214481 Zero the data allocated on stack that we pass to bpf_perf_event_output(). Observed with: - linux 5.10.14 - bcc 0.18.0 --- unixdump/unixdump/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unixdump/unixdump/__init__.py b/unixdump/unixdump/__init__.py index 3b126e3..7660d17 100644 --- a/unixdump/unixdump/__init__.py +++ b/unixdump/unixdump/__init__.py @@ -595,6 +595,7 @@ def main(): int kprobe__unix_stream_sendmsg(struct pt_regs *ctx, struct socket *sock, struct msghdr *msg){ struct notify_t n; + __builtin_memset(&n, 0, sizeof(n)); n.cpu = bpf_get_smp_processor_id(); n.type = SOCK_STREAM; @@ -920,6 +921,7 @@ def main(): int kprobe__unix_dgram_sendmsg(struct pt_regs *ctx, struct socket *sock, struct msghdr *msg){ struct notify_t n; + __builtin_memset(&n, 0, sizeof(n)); n.cpu = bpf_get_smp_processor_id(); n.type = SOCK_DGRAM; From b3aaed20cbc31105dbbaa7eb1aaad89d39115a77 Mon Sep 17 00:00:00 2001 From: Jakub Sitnicki Date: Mon, 8 Mar 2021 18:07:54 +0100 Subject: [PATCH 2/2] Guard against unbounded access in bpf_probe_read error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When starting unixdump, BPF verifier complains about unbounded access: 739: (85) call bpf_probe_read#4 … R2 unbounded memory access, use 'var &= const' or 'if (var < const)' Follow the verifier suggestion, and add a redundant check for the upper bound of the offset passed to bpf_probe_read(). Observed with: - linux 5.10.18 - bcc 0.16.17 --- unixdump/unixdump/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unixdump/unixdump/__init__.py b/unixdump/unixdump/__init__.py index 7660d17..77fe027 100644 --- a/unixdump/unixdump/__init__.py +++ b/unixdump/unixdump/__init__.py @@ -524,7 +524,9 @@ def main(): if (l >= BUFFER_SIZE) { l = BUFFER_SIZE - 1; } - bpf_probe_read(entry->buffer, l, base); + if (l < BUFFER_SIZE) { + bpf_probe_read(entry->buffer, l, base); + } } static inline struct cmsghdr* cmsg_firsthdr_x(struct msghdr* msg) {