All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: ast@kernel.org
Cc: netdev@vger.kernel.org, Daniel Borkmann <daniel@iogearbox.net>
Subject: [PATCH bpf-next 09/11] bpf: fix context access in tracing progs on 32 bit archs
Date: Mon, 28 May 2018 02:43:42 +0200	[thread overview]
Message-ID: <20180528004344.3606-10-daniel@iogearbox.net> (raw)
In-Reply-To: <20180528004344.3606-1-daniel@iogearbox.net>

Wang reported that all the testcases for BPF_PROG_TYPE_PERF_EVENT
program type in test_verifier report the following errors on x86_32:

  172/p unpriv: spill/fill of different pointers ldx FAIL
  Unexpected error message!
  0: (bf) r6 = r10
  1: (07) r6 += -8
  2: (15) if r1 == 0x0 goto pc+3
  R1=ctx(id=0,off=0,imm=0) R6=fp-8,call_-1 R10=fp0,call_-1
  3: (bf) r2 = r10
  4: (07) r2 += -76
  5: (7b) *(u64 *)(r6 +0) = r2
  6: (55) if r1 != 0x0 goto pc+1
  R1=ctx(id=0,off=0,imm=0) R2=fp-76,call_-1 R6=fp-8,call_-1 R10=fp0,call_-1 fp-8=fp
  7: (7b) *(u64 *)(r6 +0) = r1
  8: (79) r1 = *(u64 *)(r6 +0)
  9: (79) r1 = *(u64 *)(r1 +68)
  invalid bpf_context access off=68 size=8

  378/p check bpf_perf_event_data->sample_period byte load permitted FAIL
  Failed to load prog 'Permission denied'!
  0: (b7) r0 = 0
  1: (71) r0 = *(u8 *)(r1 +68)
  invalid bpf_context access off=68 size=1

  379/p check bpf_perf_event_data->sample_period half load permitted FAIL
  Failed to load prog 'Permission denied'!
  0: (b7) r0 = 0
  1: (69) r0 = *(u16 *)(r1 +68)
  invalid bpf_context access off=68 size=2

  380/p check bpf_perf_event_data->sample_period word load permitted FAIL
  Failed to load prog 'Permission denied'!
  0: (b7) r0 = 0
  1: (61) r0 = *(u32 *)(r1 +68)
  invalid bpf_context access off=68 size=4

  381/p check bpf_perf_event_data->sample_period dword load permitted FAIL
  Failed to load prog 'Permission denied'!
  0: (b7) r0 = 0
  1: (79) r0 = *(u64 *)(r1 +68)
  invalid bpf_context access off=68 size=8

Reason is that struct pt_regs on x86_32 doesn't fully align to 8 byte
boundary due to its size of 68 bytes.

Therefore, bpf_ctx_narrow_access_ok() will then bail out saying that
off & (size_default - 1) which is 68 & 7 doesn't cleanly align in the
case of sample_period access from struct bpf_perf_event_data, hence
verifier wrongly thinks we might be doing an unaligned access here.
Therefore adjust this down to machine size and check the offset for
narrow access on that basis.

We also need to fix pe_prog_is_valid_access(), since we hit the check
for off % size != 0 (e.g. 68 % 8 -> 4) in the first and last test.

Reported-by: Wang YanQing <udknight@gmail.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/filter.h   | 30 ++++++++++++++++++++++++------
 kernel/trace/bpf_trace.c | 10 ++++++++--
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index d407ede..89903d2 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -639,16 +639,34 @@ static inline bool bpf_prog_was_classic(const struct bpf_prog *prog)
 	return prog->type == BPF_PROG_TYPE_UNSPEC;
 }
 
-static inline bool
-bpf_ctx_narrow_access_ok(u32 off, u32 size, const u32 size_default)
+static inline u32 bpf_ctx_off_adjust_machine(u32 size)
+{
+	const u32 size_machine = sizeof(unsigned long);
+
+	if (size > size_machine && size % size_machine == 0)
+		size = size_machine;
+
+	return size;
+}
+
+static inline bool bpf_ctx_narrow_align_ok(u32 off, u32 size_access,
+					   u32 size_default)
 {
-	bool off_ok;
+	size_default = bpf_ctx_off_adjust_machine(size_default);
+	size_access  = bpf_ctx_off_adjust_machine(size_access);
+
 #ifdef __LITTLE_ENDIAN
-	off_ok = (off & (size_default - 1)) == 0;
+	return (off & (size_default - 1)) == 0;
 #else
-	off_ok = (off & (size_default - 1)) + size == size_default;
+	return (off & (size_default - 1)) + size_access == size_default;
 #endif
-	return off_ok && size <= size_default && (size & (size - 1)) == 0;
+}
+
+static inline bool
+bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
+{
+	return bpf_ctx_narrow_align_ok(off, size, size_default) &&
+	       size <= size_default && (size & (size - 1)) == 0;
 }
 
 #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 81fdf2f..7269530 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -880,8 +880,14 @@ static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type
 		return false;
 	if (type != BPF_READ)
 		return false;
-	if (off % size != 0)
-		return false;
+	if (off % size != 0) {
+		if (sizeof(unsigned long) != 4)
+			return false;
+		if (size != 8)
+			return false;
+		if (off % size != 4)
+			return false;
+	}
 
 	switch (off) {
 	case bpf_ctx_range(struct bpf_perf_event_data, sample_period):
-- 
2.9.5

  parent reply	other threads:[~2018-05-28  0:44 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-28  0:43 [PATCH bpf-next 00/11] Misc BPF improvements Daniel Borkmann
2018-05-28  0:43 ` [PATCH bpf-next 01/11] bpf: test case for map pointer poison with calls/branches Daniel Borkmann
2018-05-29 18:01   ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 02/11] bpf: add also cbpf long jump test cases with heavy expansion Daniel Borkmann
2018-05-29 18:09   ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 03/11] bpf: fixup error message from gpl helpers on license mismatch Daniel Borkmann
2018-05-29 17:16   ` Jesper Dangaard Brouer
2018-05-29 18:10     ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 04/11] bpf: show prog and map id in fdinfo Daniel Borkmann
2018-05-29 17:27   ` Jesper Dangaard Brouer
2018-05-29 19:55     ` Daniel Borkmann
2018-05-30 16:15       ` Song Liu
2018-05-30 17:15         ` Jesper Dangaard Brouer
2018-05-28  0:43 ` [PATCH bpf-next 05/11] bpf: avoid retpoline for lookup/update/delete calls on maps Daniel Borkmann
2018-05-29 17:23   ` Jesper Dangaard Brouer
2018-05-30 17:06   ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 06/11] bpf: add bpf_skb_cgroup_id helper Daniel Borkmann
2018-05-29 12:15   ` Quentin Monnet
2018-05-29 15:43     ` Daniel Borkmann
2018-05-28  0:43 ` [PATCH bpf-next 07/11] bpf: make sure to clear unused fields in tunnel/xfrm state fetch Daniel Borkmann
2018-05-30 17:15   ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 08/11] bpf: fix cbpf parser bug for octal numbers Daniel Borkmann
2018-05-30 17:16   ` Song Liu
2018-05-28  0:43 ` Daniel Borkmann [this message]
2018-05-30 16:46   ` [PATCH bpf-next 09/11] bpf: fix context access in tracing progs on 32 bit archs Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 10/11] bpf: sync bpf uapi header with tools Daniel Borkmann
2018-05-30 16:10   ` Song Liu
2018-05-28  0:43 ` [PATCH bpf-next 11/11] bpf, doc: add missing patchwork url and libbpf to maintainers Daniel Borkmann
2018-05-30  0:16   ` Song Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180528004344.3606-10-daniel@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=ast@kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.