* [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling @ 2021-01-12 9:14 Gilad Reti 2021-01-12 9:26 ` [PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill Gilad Reti ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Gilad Reti @ 2021-01-12 9:14 UTC (permalink / raw) To: bpf Cc: gilad.reti, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, netdev, linux-kernel Add support for pointer to mem register spilling, to allow the verifier to track pointer to valid memory addresses. Such pointers are returned for example by a successful call of the bpf_ringbuf_reserve helper. This patch was suggested as a solution by Yonghong Song. The patch was partially contibuted by CyberArk Software, Inc. Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it") Signed-off-by: Gilad Reti <gilad.reti@gmail.com> --- kernel/bpf/verifier.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 17270b8404f1..36af69fac591 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) case PTR_TO_RDWR_BUF: case PTR_TO_RDWR_BUF_OR_NULL: case PTR_TO_PERCPU_BTF_ID: + case PTR_TO_MEM: + case PTR_TO_MEM_OR_NULL: return true; default: return false; -- 2.27.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill 2021-01-12 9:14 [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling Gilad Reti @ 2021-01-12 9:26 ` Gilad Reti 2021-01-12 13:57 ` [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling KP Singh 2021-01-12 19:46 ` Andrii Nakryiko 2 siblings, 0 replies; 7+ messages in thread From: Gilad Reti @ 2021-01-12 9:26 UTC (permalink / raw) To: bpf Cc: gilad.reti, Shuah Khan, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, linux-kselftest, netdev, linux-kernel Add test to check that the verifier is able to recognize spilling of PTR_TO_MEM registers. The patch was partially contibuted by CyberArk Software, Inc. Signed-off-by: Gilad Reti <gilad.reti@gmail.com> --- tools/testing/selftests/bpf/test_verifier.c | 12 +++++++- .../selftests/bpf/verifier/spill_fill.c | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 777a81404fdb..f8569f04064b 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -50,7 +50,7 @@ #define MAX_INSNS BPF_MAXINSNS #define MAX_TEST_INSNS 1000000 #define MAX_FIXUPS 8 -#define MAX_NR_MAPS 20 +#define MAX_NR_MAPS 21 #define MAX_TEST_RUNS 8 #define POINTER_VALUE 0xcafe4all #define TEST_DATA_LEN 64 @@ -87,6 +87,7 @@ struct bpf_test { int fixup_sk_storage_map[MAX_FIXUPS]; int fixup_map_event_output[MAX_FIXUPS]; int fixup_map_reuseport_array[MAX_FIXUPS]; + int fixup_map_ringbuf[MAX_FIXUPS]; const char *errstr; const char *errstr_unpriv; uint32_t insn_processed; @@ -640,6 +641,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type, int *fixup_sk_storage_map = test->fixup_sk_storage_map; int *fixup_map_event_output = test->fixup_map_event_output; int *fixup_map_reuseport_array = test->fixup_map_reuseport_array; + int *fixup_map_ringbuf = test->fixup_map_ringbuf; if (test->fill_helper) { test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn)); @@ -817,6 +819,14 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type, fixup_map_reuseport_array++; } while (*fixup_map_reuseport_array); } + if (*fixup_map_ringbuf) { + map_fds[20] = create_map(BPF_MAP_TYPE_RINGBUF, 0, + 0, 4096); + do { + prog[*fixup_map_ringbuf].imm = map_fds[20]; + fixup_map_ringbuf++; + } while (*fixup_map_ringbuf); + } } struct libcap { diff --git a/tools/testing/selftests/bpf/verifier/spill_fill.c b/tools/testing/selftests/bpf/verifier/spill_fill.c index 45d43bf82f26..1833b6c730dd 100644 --- a/tools/testing/selftests/bpf/verifier/spill_fill.c +++ b/tools/testing/selftests/bpf/verifier/spill_fill.c @@ -28,6 +28,36 @@ .result = ACCEPT, .result_unpriv = ACCEPT, }, +{ + "check valid spill/fill, ptr to mem", + .insns = { + /* reserve 8 byte ringbuf memory */ + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0), + BPF_LD_MAP_FD(BPF_REG_1, 0), + BPF_MOV64_IMM(BPF_REG_2, 8), + BPF_MOV64_IMM(BPF_REG_3, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve), + /* store a pointer to the reserved memory in R6 */ + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0), + /* check whether the reservation was successful */ + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6), + /* spill R6(mem) into the stack */ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_6, -8), + /* fill it back in R7 */ + BPF_LDX_MEM(BPF_DW, BPF_REG_7, BPF_REG_10, -8), + /* should be able to access *(R7) = 0 */ + BPF_ST_MEM(BPF_DW, BPF_REG_7, 0, 0), + /* submit the reserved rungbuf memory */ + BPF_MOV64_REG(BPF_REG_1, BPF_REG_7), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_submit), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }, + .fixup_map_ringbuf = { 1 }, + .result = ACCEPT, + .result_unpriv = ACCEPT, +}, { "check corrupted spill/fill", .insns = { -- 2.27.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling 2021-01-12 9:14 [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling Gilad Reti 2021-01-12 9:26 ` [PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill Gilad Reti @ 2021-01-12 13:57 ` KP Singh 2021-01-12 14:23 ` Gilad Reti 2021-01-12 19:46 ` Andrii Nakryiko 2 siblings, 1 reply; 7+ messages in thread From: KP Singh @ 2021-01-12 13:57 UTC (permalink / raw) To: Gilad Reti Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, Networking, open list On Tue, Jan 12, 2021 at 10:14 AM Gilad Reti <gilad.reti@gmail.com> wrote: > > Add support for pointer to mem register spilling, to allow the verifier > to track pointer to valid memory addresses. Such pointers are returned nit: pointers > for example by a successful call of the bpf_ringbuf_reserve helper. > > This patch was suggested as a solution by Yonghong Song. You can use the "Suggested-by:" tag for this. > > The patch was partially contibuted by CyberArk Software, Inc. nit: typo *contributed Also, I was wondering if "partially" here means someone collaborated with you on the patch? And, in that case: "Co-developed-by:" would be a better tag here. Acked-by: KP Singh <kpsingh@kernel.org> > > Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier > support for it") > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > --- > kernel/bpf/verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 17270b8404f1..36af69fac591 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > case PTR_TO_RDWR_BUF: > case PTR_TO_RDWR_BUF_OR_NULL: > case PTR_TO_PERCPU_BTF_ID: > + case PTR_TO_MEM: > + case PTR_TO_MEM_OR_NULL: > return true; > default: > return false; > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling 2021-01-12 13:57 ` [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling KP Singh @ 2021-01-12 14:23 ` Gilad Reti 2021-01-12 15:01 ` KP Singh 0 siblings, 1 reply; 7+ messages in thread From: Gilad Reti @ 2021-01-12 14:23 UTC (permalink / raw) To: KP Singh Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, Networking, open list On Tue, Jan 12, 2021 at 3:57 PM KP Singh <kpsingh@kernel.org> wrote: > > On Tue, Jan 12, 2021 at 10:14 AM Gilad Reti <gilad.reti@gmail.com> wrote: > > > > Add support for pointer to mem register spilling, to allow the verifier > > to track pointer to valid memory addresses. Such pointers are returned > > nit: pointers Thanks > > > for example by a successful call of the bpf_ringbuf_reserve helper. > > > > This patch was suggested as a solution by Yonghong Song. > > You can use the "Suggested-by:" tag for this. Thanks > > > > > The patch was partially contibuted by CyberArk Software, Inc. > > nit: typo *contributed Thanks. Should I submit a v2 of the patch to correct all of those? > > Also, I was wondering if "partially" here means someone collaborated with you > on the patch? And, in that case: > > "Co-developed-by:" would be a better tag here. No, I did it alone. I mentioned CyberArk since I work there and did some of the coding during my daily work, so they deserve credit. > > Acked-by: KP Singh <kpsingh@kernel.org> > > > > > > Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier > > support for it") > > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > > --- > > kernel/bpf/verifier.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index 17270b8404f1..36af69fac591 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > > case PTR_TO_RDWR_BUF: > > case PTR_TO_RDWR_BUF_OR_NULL: > > case PTR_TO_PERCPU_BTF_ID: > > + case PTR_TO_MEM: > > + case PTR_TO_MEM_OR_NULL: > > return true; > > default: > > return false; > > -- > > 2.27.0 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling 2021-01-12 14:23 ` Gilad Reti @ 2021-01-12 15:01 ` KP Singh 0 siblings, 0 replies; 7+ messages in thread From: KP Singh @ 2021-01-12 15:01 UTC (permalink / raw) To: Gilad Reti Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, Networking, open list On Tue, Jan 12, 2021 at 3:24 PM Gilad Reti <gilad.reti@gmail.com> wrote: > > On Tue, Jan 12, 2021 at 3:57 PM KP Singh <kpsingh@kernel.org> wrote: > > > > On Tue, Jan 12, 2021 at 10:14 AM Gilad Reti <gilad.reti@gmail.com> wrote: > > > > > > Add support for pointer to mem register spilling, to allow the verifier > > > to track pointer to valid memory addresses. Such pointers are returned > > > > nit: pointers > > Thanks > > > > > > for example by a successful call of the bpf_ringbuf_reserve helper. > > > > > > This patch was suggested as a solution by Yonghong Song. > > > > You can use the "Suggested-by:" tag for this. > > Thanks > > > > > > > > > The patch was partially contibuted by CyberArk Software, Inc. > > > > nit: typo *contributed > > Thanks. Should I submit a v2 of the patch to correct all of those? I think it would be nice to do another revision which also addresses the comments on the other patch. > > > > > Also, I was wondering if "partially" here means someone collaborated with you > > on the patch? And, in that case: > > > > "Co-developed-by:" would be a better tag here. > > No, I did it alone. I mentioned CyberArk since I work there and did some of the > coding during my daily work, so they deserve credit. > > > > > Acked-by: KP Singh <kpsingh@kernel.org> > > > > > > > > > > Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier > > > support for it") > > > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > > > --- > > > kernel/bpf/verifier.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > > index 17270b8404f1..36af69fac591 100644 > > > --- a/kernel/bpf/verifier.c > > > +++ b/kernel/bpf/verifier.c > > > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > > > case PTR_TO_RDWR_BUF: > > > case PTR_TO_RDWR_BUF_OR_NULL: > > > case PTR_TO_PERCPU_BTF_ID: > > > + case PTR_TO_MEM: > > > + case PTR_TO_MEM_OR_NULL: > > > return true; > > > default: > > > return false; > > > -- > > > 2.27.0 > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling 2021-01-12 9:14 [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling Gilad Reti 2021-01-12 9:26 ` [PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill Gilad Reti 2021-01-12 13:57 ` [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling KP Singh @ 2021-01-12 19:46 ` Andrii Nakryiko 2021-01-12 20:21 ` Daniel Borkmann 2 siblings, 1 reply; 7+ messages in thread From: Andrii Nakryiko @ 2021-01-12 19:46 UTC (permalink / raw) To: Gilad Reti Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Networking, open list On Tue, Jan 12, 2021 at 1:14 AM Gilad Reti <gilad.reti@gmail.com> wrote: > > Add support for pointer to mem register spilling, to allow the verifier > to track pointer to valid memory addresses. Such pointers are returned > for example by a successful call of the bpf_ringbuf_reserve helper. > > This patch was suggested as a solution by Yonghong Song. > > The patch was partially contibuted by CyberArk Software, Inc. > > Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier > support for it") Surprised no one mentioned this yet. Fixes tag should always be on a single unwrapped line, however long it is, please fix. > Signed-off-by: Gilad Reti <gilad.reti@gmail.com> > --- > kernel/bpf/verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 17270b8404f1..36af69fac591 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -2217,6 +2217,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type) > case PTR_TO_RDWR_BUF: > case PTR_TO_RDWR_BUF_OR_NULL: > case PTR_TO_PERCPU_BTF_ID: > + case PTR_TO_MEM: > + case PTR_TO_MEM_OR_NULL: > return true; > default: > return false; > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling 2021-01-12 19:46 ` Andrii Nakryiko @ 2021-01-12 20:21 ` Daniel Borkmann 0 siblings, 0 replies; 7+ messages in thread From: Daniel Borkmann @ 2021-01-12 20:21 UTC (permalink / raw) To: Andrii Nakryiko, Gilad Reti Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Networking, open list On 1/12/21 8:46 PM, Andrii Nakryiko wrote: > On Tue, Jan 12, 2021 at 1:14 AM Gilad Reti <gilad.reti@gmail.com> wrote: >> >> Add support for pointer to mem register spilling, to allow the verifier >> to track pointer to valid memory addresses. Such pointers are returned >> for example by a successful call of the bpf_ringbuf_reserve helper. >> >> This patch was suggested as a solution by Yonghong Song. >> >> The patch was partially contibuted by CyberArk Software, Inc. >> >> Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier >> support for it") > > Surprised no one mentioned this yet. Fixes tag should always be on a > single unwrapped line, however long it is, please fix. Especially for first-time contributors there is usually some luxury that we would have fixed this up on the fly while applying. ;) But given a v2 is going to be sent anyway it's good to point it out for future reference, agree. Thanks, Daniel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-12 21:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-12 9:14 [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling Gilad Reti 2021-01-12 9:26 ` [PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill Gilad Reti 2021-01-12 13:57 ` [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling KP Singh 2021-01-12 14:23 ` Gilad Reti 2021-01-12 15:01 ` KP Singh 2021-01-12 19:46 ` Andrii Nakryiko 2021-01-12 20:21 ` 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).