Re: [PATCH bpf v2 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill

2021-01-13 Thread KP Singh
On Wed, Jan 13, 2021 at 5:05 PM Yonghong Song  wrote:
>
>
>
> On 1/12/21 9:38 PM, Gilad Reti wrote:
> > 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 
>
> I didn't verify result_unpriv = ACCEPT part. I think it is correct
> by checking code.
>
> Acked-by: Yonghong Song 

Thanks for the description!

Acked-by: KP Singh 


Re: [PATCH bpf v2 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill

2021-01-13 Thread Yonghong Song




On 1/12/21 9:38 PM, Gilad Reti wrote:

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 


I didn't verify result_unpriv = ACCEPT part. I think it is correct
by checking code.

Acked-by: Yonghong Song 


[PATCH bpf v2 2/2] selftests/bpf: add verifier test for PTR_TO_MEM spill

2021-01-12 Thread Gilad Reti
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