[PATCH bpf v2 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill
Add a test to check that the verifier is able to recognize spilling of
PTR_TO_MEM registers, by reserving a ringbuf buffer, forcing the spill
of a pointer holding the buffer address to the stack, filling it back
in from the stack and writing to the memory area pointed by it.
The patch was partially contributed by CyberArk Software, Inc.
Signed-off-by: Gilad Reti
---
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 100
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS20
+#define MAX_NR_MAPS21
#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..0b943897aaf6 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 ringbuf 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
[PATCH bpf v2 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling
Add support for pointer to mem register spilling, to allow the verifier
to track pointers to valid memory addresses. Such pointers are returned
for example by a successful call of the bpf_ringbuf_reserve helper.
The patch was partially contributed by CyberArk Software, Inc.
Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for
it")
Suggested-by: Yonghong Song
Signed-off-by: Gilad Reti
---
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
Re: [PATCH 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill
On Tue, Jan 12, 2021 at 6:17 PM KP Singh wrote: > > On Tue, Jan 12, 2021 at 4:43 PM Daniel Borkmann wrote: > > > > On 1/12/21 4:35 PM, Gilad Reti wrote: > > > On Tue, Jan 12, 2021 at 4:56 PM KP Singh wrote: > > >> On Tue, Jan 12, 2021 at 10:16 AM Gilad Reti wrote: > > >>> > > >>> Add test to check that the verifier is able to recognize spilling of > > >>> PTR_TO_MEM registers. > > >> > > >> It would be nice to have some explanation of what the test does to > > >> recognize the spilling of the PTR_TO_MEM registers in the commit > > >> log as well. > > >> > > >> Would it be possible to augment an existing test_progs > > >> program like tools/testing/selftests/bpf/progs/test_ringbuf.c to test > > >> this functionality? > > > > How would you guarantee that LLVM generates the spill/fill, via inline asm? > > Yeah, I guess there is no sure-shot way to do it and, adding inline asm would > just be doing the same thing as this verifier test. You can ignore me > on this one :) > > It would, however, be nice to have a better description about what the test is > actually doing./ > > I will re-submit the patch tomorrow. Thank you all for your patience. > > > > > It may be possible, but from what I understood from Daniel's comment here > > > > > > https://lore.kernel.org/bpf/17629073-4fab-a922-ecc3-25b019960...@iogearbox.net/ > > > > > > the test should be a part of the verifier tests (which is reasonable > > > to me since it is > > > a verifier bugfix) > > > > Yeah, the test_verifier case as you have is definitely the most straight > > forward way to add coverage in this case.
Re: [PATCH 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill
On Tue, Jan 12, 2021 at 4:56 PM KP Singh wrote:
>
> On Tue, Jan 12, 2021 at 10:16 AM Gilad Reti wrote:
> >
> > Add test to check that the verifier is able to recognize spilling of
> > PTR_TO_MEM registers.
> >
>
> It would be nice to have some explanation of what the test does to
> recognize the spilling of the PTR_TO_MEM registers in the commit
> log as well.
>
> Would it be possible to augment an existing test_progs
> program like tools/testing/selftests/bpf/progs/test_ringbuf.c to test
> this functionality?
>
It may be possible, but from what I understood from Daniel's comment here
https://lore.kernel.org/bpf/17629073-4fab-a922-ecc3-25b019960...@iogearbox.net/
the test should be a part of the verifier tests (which is reasonable
to me since it is
a verifier bugfix)
>
>
> > The patch was partially contibuted by CyberArk Software, Inc.
> >
> > Signed-off-by: Gilad Reti
> > ---
> > 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 100
> > #define MAX_FIXUPS 8
> > -#define MAX_NR_MAPS20
> > +#define MAX_NR_MAPS21
> > #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
> >
Re: [PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling
On Tue, Jan 12, 2021 at 3:57 PM KP Singh wrote:
>
> On Tue, Jan 12, 2021 at 10:14 AM Gilad Reti 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
>
>
> >
> > Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier
> > support for it")
> > Signed-off-by: Gilad Reti
> > ---
> > 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
> >
[PATCH bpf 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill
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
---
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 100
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS20
+#define MAX_NR_MAPS21
#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
[PATCH 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill
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
---
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 100
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS20
+#define MAX_NR_MAPS21
#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
[PATCH bpf 1/2] bpf: support PTR_TO_MEM{,_OR_NULL} register spilling
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
---
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
Re: [PATCH] Signed-off-by: giladreti
On Mon, Jan 11, 2021, 17:55 Daniel Borkmann wrote: > > Hello Gilad, > > On 1/11/21 4:31 PM, giladreti wrote: > > Added 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 SoB should not be in subject line but as part of the commit message > instead > and with proper name, e.g. > > Signed-off-by: Gilad Reti > > For subject line, please use a short summary that fits the patch prefixed with > the subsystem "bpf: [...]", see also [0] as an example. Thanks. > > It would be good if you could also add a BPF selftest for this [1]. > >[0] > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc >[1] > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/ > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/verifier/spill_fill.c > Sure. Thanks for your guidance. As you can probably tell, I am new to kernel code contribution (in fact this is a first time for me). Should I try to submit this patch again? Sorry in advance for all the overhead I may be causing to you... > > --- > > 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; > > >
