* [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform @ 2018-04-26 9:57 Wang YanQing 2018-04-27 22:48 ` Alexei Starovoitov 0 siblings, 1 reply; 6+ messages in thread From: Wang YanQing @ 2018-04-26 9:57 UTC (permalink / raw) To: daniel; +Cc: ast, netdev, linux-kernel All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in test_verifier(kselftest) report below 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 " This patch fix it, the fix isn't only necessary for x86_32, it will fix the same problem for other platforms too, if their size of bpf_user_pt_regs_t can't divide exactly into 8. Signed-off-by: Wang YanQing <udknight@gmail.com> --- Hi all! After mainline accept this patch, then we need to submit a sync patch to update the tools/include/uapi/linux/bpf_perf_event.h. Thanks. include/uapi/linux/bpf_perf_event.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h index eb1b9d2..ff4c092 100644 --- a/include/uapi/linux/bpf_perf_event.h +++ b/include/uapi/linux/bpf_perf_event.h @@ -12,7 +12,7 @@ struct bpf_perf_event_data { bpf_user_pt_regs_t regs; - __u64 sample_period; + __u64 sample_period __attribute__((aligned(8))); __u64 addr; }; -- 1.8.5.6.2.g3d8a54e.dirty ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform 2018-04-26 9:57 [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform Wang YanQing @ 2018-04-27 22:48 ` Alexei Starovoitov 2018-04-27 23:33 ` Daniel Borkmann 0 siblings, 1 reply; 6+ messages in thread From: Alexei Starovoitov @ 2018-04-27 22:48 UTC (permalink / raw) To: Wang YanQing, daniel, ast, netdev, linux-kernel On Thu, Apr 26, 2018 at 05:57:49PM +0800, Wang YanQing wrote: > All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in > test_verifier(kselftest) report below 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 > " > > This patch fix it, the fix isn't only necessary for x86_32, it will fix the > same problem for other platforms too, if their size of bpf_user_pt_regs_t > can't divide exactly into 8. > > Signed-off-by: Wang YanQing <udknight@gmail.com> > --- > Hi all! > After mainline accept this patch, then we need to submit a sync patch > to update the tools/include/uapi/linux/bpf_perf_event.h. > > Thanks. > > include/uapi/linux/bpf_perf_event.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h > index eb1b9d2..ff4c092 100644 > --- a/include/uapi/linux/bpf_perf_event.h > +++ b/include/uapi/linux/bpf_perf_event.h > @@ -12,7 +12,7 @@ > > struct bpf_perf_event_data { > bpf_user_pt_regs_t regs; > - __u64 sample_period; > + __u64 sample_period __attribute__((aligned(8))); I don't think this necessary. imo it's a bug in pe_prog_is_valid_access that should have allowed 8-byte access to 4-byte aligned sample_period. The access rewritten by pe_prog_convert_ctx_access anyway, no alignment issues as far as I can see. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform 2018-04-27 22:48 ` Alexei Starovoitov @ 2018-04-27 23:33 ` Daniel Borkmann 2018-04-28 5:29 ` Wang YanQing 0 siblings, 1 reply; 6+ messages in thread From: Daniel Borkmann @ 2018-04-27 23:33 UTC (permalink / raw) To: Alexei Starovoitov, Wang YanQing, ast, netdev, linux-kernel On 04/28/2018 12:48 AM, Alexei Starovoitov wrote: > On Thu, Apr 26, 2018 at 05:57:49PM +0800, Wang YanQing wrote: >> All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in >> test_verifier(kselftest) report below 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 >> " >> >> This patch fix it, the fix isn't only necessary for x86_32, it will fix the >> same problem for other platforms too, if their size of bpf_user_pt_regs_t >> can't divide exactly into 8. >> >> Signed-off-by: Wang YanQing <udknight@gmail.com> >> --- >> Hi all! >> After mainline accept this patch, then we need to submit a sync patch >> to update the tools/include/uapi/linux/bpf_perf_event.h. >> >> Thanks. >> >> include/uapi/linux/bpf_perf_event.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h >> index eb1b9d2..ff4c092 100644 >> --- a/include/uapi/linux/bpf_perf_event.h >> +++ b/include/uapi/linux/bpf_perf_event.h >> @@ -12,7 +12,7 @@ >> >> struct bpf_perf_event_data { >> bpf_user_pt_regs_t regs; >> - __u64 sample_period; >> + __u64 sample_period __attribute__((aligned(8))); > > I don't think this necessary. > imo it's a bug in pe_prog_is_valid_access > that should have allowed 8-byte access to 4-byte aligned sample_period. > The access rewritten by pe_prog_convert_ctx_access anyway, > no alignment issues as far as I can see. Right, good point. Wang, could you give the below a test run: diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 56ba0f2..95b9142 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -833,8 +833,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(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): ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform 2018-04-27 23:33 ` Daniel Borkmann @ 2018-04-28 5:29 ` Wang YanQing 2018-05-07 7:23 ` Wang YanQing 0 siblings, 1 reply; 6+ messages in thread From: Wang YanQing @ 2018-04-28 5:29 UTC (permalink / raw) To: Daniel Borkmann; +Cc: Alexei Starovoitov, ast, netdev, linux-kernel On Sat, Apr 28, 2018 at 01:33:15AM +0200, Daniel Borkmann wrote: > On 04/28/2018 12:48 AM, Alexei Starovoitov wrote: > > On Thu, Apr 26, 2018 at 05:57:49PM +0800, Wang YanQing wrote: > >> All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in > >> test_verifier(kselftest) report below 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 > >> " > >> > >> This patch fix it, the fix isn't only necessary for x86_32, it will fix the > >> same problem for other platforms too, if their size of bpf_user_pt_regs_t > >> can't divide exactly into 8. > >> > >> Signed-off-by: Wang YanQing <udknight@gmail.com> > >> --- > >> Hi all! > >> After mainline accept this patch, then we need to submit a sync patch > >> to update the tools/include/uapi/linux/bpf_perf_event.h. > >> > >> Thanks. > >> > >> include/uapi/linux/bpf_perf_event.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h > >> index eb1b9d2..ff4c092 100644 > >> --- a/include/uapi/linux/bpf_perf_event.h > >> +++ b/include/uapi/linux/bpf_perf_event.h > >> @@ -12,7 +12,7 @@ > >> > >> struct bpf_perf_event_data { > >> bpf_user_pt_regs_t regs; > >> - __u64 sample_period; > >> + __u64 sample_period __attribute__((aligned(8))); > > > > I don't think this necessary. > > imo it's a bug in pe_prog_is_valid_access > > that should have allowed 8-byte access to 4-byte aligned sample_period. > > The access rewritten by pe_prog_convert_ctx_access anyway, > > no alignment issues as far as I can see. > > Right, good point. Wang, could you give the below a test run: > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 56ba0f2..95b9142 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -833,8 +833,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(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): Hi all! I have tested this patch, but test_verifier reports the same errors for the five testcases. The reason is they all failed to pass the test of bpf_ctx_narrow_access_ok. Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform 2018-04-28 5:29 ` Wang YanQing @ 2018-05-07 7:23 ` Wang YanQing 2018-05-07 8:25 ` Daniel Borkmann 0 siblings, 1 reply; 6+ messages in thread From: Wang YanQing @ 2018-05-07 7:23 UTC (permalink / raw) To: Daniel Borkmann, Alexei Starovoitov, ast, netdev, linux-kernel On Sat, Apr 28, 2018 at 01:29:17PM +0800, Wang YanQing wrote: > On Sat, Apr 28, 2018 at 01:33:15AM +0200, Daniel Borkmann wrote: > > On 04/28/2018 12:48 AM, Alexei Starovoitov wrote: > > > On Thu, Apr 26, 2018 at 05:57:49PM +0800, Wang YanQing wrote: > > >> All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in > > >> test_verifier(kselftest) report below 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 > > >> " > > >> > > >> This patch fix it, the fix isn't only necessary for x86_32, it will fix the > > >> same problem for other platforms too, if their size of bpf_user_pt_regs_t > > >> can't divide exactly into 8. > > >> > > >> Signed-off-by: Wang YanQing <udknight@gmail.com> > > >> --- > > >> Hi all! > > >> After mainline accept this patch, then we need to submit a sync patch > > >> to update the tools/include/uapi/linux/bpf_perf_event.h. > > >> > > >> Thanks. > > >> > > >> include/uapi/linux/bpf_perf_event.h | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h > > >> index eb1b9d2..ff4c092 100644 > > >> --- a/include/uapi/linux/bpf_perf_event.h > > >> +++ b/include/uapi/linux/bpf_perf_event.h > > >> @@ -12,7 +12,7 @@ > > >> > > >> struct bpf_perf_event_data { > > >> bpf_user_pt_regs_t regs; > > >> - __u64 sample_period; > > >> + __u64 sample_period __attribute__((aligned(8))); > > > > > > I don't think this necessary. > > > imo it's a bug in pe_prog_is_valid_access > > > that should have allowed 8-byte access to 4-byte aligned sample_period. > > > The access rewritten by pe_prog_convert_ctx_access anyway, > > > no alignment issues as far as I can see. > > > > Right, good point. Wang, could you give the below a test run: > > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > > index 56ba0f2..95b9142 100644 > > --- a/kernel/trace/bpf_trace.c > > +++ b/kernel/trace/bpf_trace.c > > @@ -833,8 +833,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(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): > Hi all! > > I have tested this patch, but test_verifier reports the same errors > for the five testcases. > > The reason is they all failed to pass the test of bpf_ctx_narrow_access_ok. > > Thanks. Hi! Daniel Borkmann. Do you have any plan to fix bpf_ctx_narrow_access_ok for these problems? Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform 2018-05-07 7:23 ` Wang YanQing @ 2018-05-07 8:25 ` Daniel Borkmann 0 siblings, 0 replies; 6+ messages in thread From: Daniel Borkmann @ 2018-05-07 8:25 UTC (permalink / raw) To: Wang YanQing, Alexei Starovoitov, ast, netdev, linux-kernel On 05/07/2018 09:23 AM, Wang YanQing wrote: > On Sat, Apr 28, 2018 at 01:29:17PM +0800, Wang YanQing wrote: >> On Sat, Apr 28, 2018 at 01:33:15AM +0200, Daniel Borkmann wrote: >>> On 04/28/2018 12:48 AM, Alexei Starovoitov wrote: >>>> On Thu, Apr 26, 2018 at 05:57:49PM +0800, Wang YanQing wrote: >>>>> All the testcases for BPF_PROG_TYPE_PERF_EVENT program type in >>>>> test_verifier(kselftest) report below 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 >>>>> " >>>>> >>>>> This patch fix it, the fix isn't only necessary for x86_32, it will fix the >>>>> same problem for other platforms too, if their size of bpf_user_pt_regs_t >>>>> can't divide exactly into 8. >>>>> >>>>> Signed-off-by: Wang YanQing <udknight@gmail.com> >>>>> --- >>>>> Hi all! >>>>> After mainline accept this patch, then we need to submit a sync patch >>>>> to update the tools/include/uapi/linux/bpf_perf_event.h. >>>>> >>>>> Thanks. >>>>> >>>>> include/uapi/linux/bpf_perf_event.h | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/include/uapi/linux/bpf_perf_event.h b/include/uapi/linux/bpf_perf_event.h >>>>> index eb1b9d2..ff4c092 100644 >>>>> --- a/include/uapi/linux/bpf_perf_event.h >>>>> +++ b/include/uapi/linux/bpf_perf_event.h >>>>> @@ -12,7 +12,7 @@ >>>>> >>>>> struct bpf_perf_event_data { >>>>> bpf_user_pt_regs_t regs; >>>>> - __u64 sample_period; >>>>> + __u64 sample_period __attribute__((aligned(8))); >>>> >>>> I don't think this necessary. >>>> imo it's a bug in pe_prog_is_valid_access >>>> that should have allowed 8-byte access to 4-byte aligned sample_period. >>>> The access rewritten by pe_prog_convert_ctx_access anyway, >>>> no alignment issues as far as I can see. >>> >>> Right, good point. Wang, could you give the below a test run: >>> >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c >>> index 56ba0f2..95b9142 100644 >>> --- a/kernel/trace/bpf_trace.c >>> +++ b/kernel/trace/bpf_trace.c >>> @@ -833,8 +833,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(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): >> Hi all! >> >> I have tested this patch, but test_verifier reports the same errors >> for the five testcases. >> >> The reason is they all failed to pass the test of bpf_ctx_narrow_access_ok. >> >> Thanks. > Hi! Daniel Borkmann. > > Do you have any plan to fix bpf_ctx_narrow_access_ok for these problems? Yep, sorry for the delay, will get to it during this week. Thanks, Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-07 8:25 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-26 9:57 [PATCH] bpf: fix misaligned access for BPF_PROG_TYPE_PERF_EVENT program type on x86_32 platform Wang YanQing 2018-04-27 22:48 ` Alexei Starovoitov 2018-04-27 23:33 ` Daniel Borkmann 2018-04-28 5:29 ` Wang YanQing 2018-05-07 7:23 ` Wang YanQing 2018-05-07 8:25 ` Daniel Borkmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).