From: Song Liu <liu.song.a23@gmail.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>, netdev@vger.kernel.org
Subject: Re: [PATCH bpf-next 01/11] bpf: test case for map pointer poison with calls/branches
Date: Tue, 29 May 2018 11:01:44 -0700 [thread overview]
Message-ID: <CAPhsuW6Oo804YHfformbyNEV=CgFa5sSC0kOOarcHcHG9ysDXw@mail.gmail.com> (raw)
In-Reply-To: <20180528004344.3606-2-daniel@iogearbox.net>
On Sun, May 27, 2018 at 5:43 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Add several test cases where the same or different map pointers
> originate from different paths in the program and execute a map
> lookup or tail call at a common location.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> include/linux/filter.h | 10 ++
> tools/include/linux/filter.h | 10 ++
> tools/testing/selftests/bpf/test_verifier.c | 185 ++++++++++++++++++++++++----
> 3 files changed, 178 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index d358d18..b443f70 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -289,6 +289,16 @@ struct xdp_buff;
> .off = OFF, \
> .imm = 0 })
>
> +/* Relative call */
> +
> +#define BPF_CALL_REL(TGT) \
> + ((struct bpf_insn) { \
> + .code = BPF_JMP | BPF_CALL, \
> + .dst_reg = 0, \
> + .src_reg = BPF_PSEUDO_CALL, \
> + .off = 0, \
> + .imm = TGT })
> +
> /* Function call */
>
> #define BPF_EMIT_CALL(FUNC) \
> diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
> index c5e512d..af55acf 100644
> --- a/tools/include/linux/filter.h
> +++ b/tools/include/linux/filter.h
> @@ -263,6 +263,16 @@
> #define BPF_LD_MAP_FD(DST, MAP_FD) \
> BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
>
> +/* Relative call */
> +
> +#define BPF_CALL_REL(TGT) \
> + ((struct bpf_insn) { \
> + .code = BPF_JMP | BPF_CALL, \
> + .dst_reg = 0, \
> + .src_reg = BPF_PSEUDO_CALL, \
> + .off = 0, \
> + .imm = TGT })
> +
> /* Program exit */
>
> #define BPF_EXIT_INSN() \
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index 4b4f015..7cb1d74 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_FIXUPS 8
> -#define MAX_NR_MAPS 4
> +#define MAX_NR_MAPS 7
> #define POINTER_VALUE 0xcafe4all
> #define TEST_DATA_LEN 64
>
> @@ -66,7 +66,9 @@ struct bpf_test {
> int fixup_map1[MAX_FIXUPS];
> int fixup_map2[MAX_FIXUPS];
> int fixup_map3[MAX_FIXUPS];
> - int fixup_prog[MAX_FIXUPS];
> + int fixup_map4[MAX_FIXUPS];
> + int fixup_prog1[MAX_FIXUPS];
> + int fixup_prog2[MAX_FIXUPS];
> int fixup_map_in_map[MAX_FIXUPS];
> const char *errstr;
> const char *errstr_unpriv;
> @@ -2769,7 +2771,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .errstr_unpriv = "R3 leaks addr into helper",
> .result_unpriv = REJECT,
> .result = ACCEPT,
> @@ -2856,7 +2858,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 1),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .result = ACCEPT,
> .retval = 42,
> },
> @@ -2870,7 +2872,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 1),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .result = ACCEPT,
> .retval = 41,
> },
> @@ -2884,7 +2886,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 1),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .result = ACCEPT,
> .retval = 1,
> },
> @@ -2898,7 +2900,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 2),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .result = ACCEPT,
> .retval = 2,
> },
> @@ -2912,7 +2914,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 2),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 1 },
> + .fixup_prog1 = { 1 },
> .result = ACCEPT,
> .retval = 2,
> },
> @@ -2926,7 +2928,7 @@ static struct bpf_test tests[] = {
> BPF_MOV64_IMM(BPF_REG_0, 2),
> BPF_EXIT_INSN(),
> },
> - .fixup_prog = { 2 },
> + .fixup_prog1 = { 2 },
> .result = ACCEPT,
> .retval = 42,
> },
> @@ -11682,6 +11684,112 @@ static struct bpf_test tests[] = {
> .prog_type = BPF_PROG_TYPE_XDP,
> },
> {
> + "calls: two calls returning different map pointers for lookup (hash, array)",
> + .insns = {
> + /* main prog */
> + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
> + BPF_CALL_REL(11),
> + BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> + BPF_CALL_REL(12),
> + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> + BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
> + offsetof(struct test_val, foo)),
> + BPF_MOV64_IMM(BPF_REG_0, 1),
> + BPF_EXIT_INSN(),
> + /* subprog 1 */
> + BPF_LD_MAP_FD(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + /* subprog 2 */
> + BPF_LD_MAP_FD(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .fixup_map2 = { 13 },
> + .fixup_map4 = { 16 },
> + .result = ACCEPT,
> + .retval = 1,
> + },
> + {
> + "calls: two calls returning different map pointers for lookup (hash, map in map)",
> + .insns = {
> + /* main prog */
> + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
> + BPF_CALL_REL(11),
> + BPF_JMP_IMM(BPF_JA, 0, 0, 1),
> + BPF_CALL_REL(12),
> + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
> + BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
> + offsetof(struct test_val, foo)),
> + BPF_MOV64_IMM(BPF_REG_0, 1),
> + BPF_EXIT_INSN(),
> + /* subprog 1 */
> + BPF_LD_MAP_FD(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + /* subprog 2 */
> + BPF_LD_MAP_FD(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .fixup_map_in_map = { 16 },
> + .fixup_map4 = { 13 },
> + .result = REJECT,
> + .errstr = "R0 invalid mem access 'map_ptr'",
> + },
> + {
> + "cond: two branches returning different map pointers for lookup (tail, tail)",
> + .insns = {
> + BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
> + offsetof(struct __sk_buff, mark)),
> + BPF_JMP_IMM(BPF_JNE, BPF_REG_6, 0, 3),
> + BPF_LD_MAP_FD(BPF_REG_2, 0),
> + BPF_JMP_IMM(BPF_JA, 0, 0, 2),
> + BPF_LD_MAP_FD(BPF_REG_2, 0),
> + BPF_MOV64_IMM(BPF_REG_3, 7),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_tail_call),
> + BPF_MOV64_IMM(BPF_REG_0, 1),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_prog1 = { 5 },
> + .fixup_prog2 = { 2 },
> + .result_unpriv = REJECT,
> + .errstr_unpriv = "tail_call abusing map_ptr",
> + .result = ACCEPT,
> + .retval = 42,
> + },
> + {
> + "cond: two branches returning same map pointers for lookup (tail, tail)",
> + .insns = {
> + BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
> + offsetof(struct __sk_buff, mark)),
> + BPF_JMP_IMM(BPF_JEQ, BPF_REG_6, 0, 3),
> + BPF_LD_MAP_FD(BPF_REG_2, 0),
> + BPF_JMP_IMM(BPF_JA, 0, 0, 2),
> + BPF_LD_MAP_FD(BPF_REG_2, 0),
> + BPF_MOV64_IMM(BPF_REG_3, 7),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_tail_call),
> + BPF_MOV64_IMM(BPF_REG_0, 1),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_prog2 = { 2, 5 },
> + .result_unpriv = ACCEPT,
> + .result = ACCEPT,
> + .retval = 42,
> + },
> + {
> "search pruning: all branches should be verified (nop operation)",
> .insns = {
> BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> @@ -12162,12 +12270,13 @@ static int probe_filter_length(const struct bpf_insn *fp)
> return len + 1;
> }
>
> -static int create_map(uint32_t size_value, uint32_t max_elem)
> +static int create_map(uint32_t type, uint32_t size_key,
> + uint32_t size_value, uint32_t max_elem)
> {
> int fd;
>
> - fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> - size_value, max_elem, BPF_F_NO_PREALLOC);
> + fd = bpf_create_map(type, size_key, size_value, max_elem,
> + type == BPF_MAP_TYPE_HASH ? BPF_F_NO_PREALLOC : 0);
> if (fd < 0)
> printf("Failed to create hash map '%s'!\n", strerror(errno));
>
> @@ -12200,13 +12309,13 @@ static int create_prog_dummy2(int mfd, int idx)
> ARRAY_SIZE(prog), "GPL", 0, NULL, 0);
> }
>
> -static int create_prog_array(void)
> +static int create_prog_array(uint32_t max_elem, int p1key)
> {
> - int p1key = 0, p2key = 1;
> + int p2key = 1;
> int mfd, p1fd, p2fd;
>
> mfd = bpf_create_map(BPF_MAP_TYPE_PROG_ARRAY, sizeof(int),
> - sizeof(int), 4, 0);
> + sizeof(int), max_elem, 0);
> if (mfd < 0) {
> printf("Failed to create prog array '%s'!\n", strerror(errno));
> return -1;
> @@ -12261,7 +12370,9 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> int *fixup_map1 = test->fixup_map1;
> int *fixup_map2 = test->fixup_map2;
> int *fixup_map3 = test->fixup_map3;
> - int *fixup_prog = test->fixup_prog;
> + int *fixup_map4 = test->fixup_map4;
> + int *fixup_prog1 = test->fixup_prog1;
> + int *fixup_prog2 = test->fixup_prog2;
> int *fixup_map_in_map = test->fixup_map_in_map;
>
> if (test->fill_helper)
> @@ -12272,7 +12383,8 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> * that really matters is value size in this case.
> */
> if (*fixup_map1) {
> - map_fds[0] = create_map(sizeof(long long), 1);
> + map_fds[0] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> + sizeof(long long), 1);
> do {
> prog[*fixup_map1].imm = map_fds[0];
> fixup_map1++;
> @@ -12280,7 +12392,8 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> }
>
> if (*fixup_map2) {
> - map_fds[1] = create_map(sizeof(struct test_val), 1);
> + map_fds[1] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> + sizeof(struct test_val), 1);
> do {
> prog[*fixup_map2].imm = map_fds[1];
> fixup_map2++;
> @@ -12288,25 +12401,43 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> }
>
> if (*fixup_map3) {
> - map_fds[1] = create_map(sizeof(struct other_val), 1);
> + map_fds[2] = create_map(BPF_MAP_TYPE_HASH, sizeof(long long),
> + sizeof(struct other_val), 1);
> do {
> - prog[*fixup_map3].imm = map_fds[1];
> + prog[*fixup_map3].imm = map_fds[2];
> fixup_map3++;
> } while (*fixup_map3);
> }
>
> - if (*fixup_prog) {
> - map_fds[2] = create_prog_array();
> + if (*fixup_map4) {
> + map_fds[3] = create_map(BPF_MAP_TYPE_ARRAY, sizeof(int),
> + sizeof(struct test_val), 1);
> + do {
> + prog[*fixup_map4].imm = map_fds[3];
> + fixup_map4++;
> + } while (*fixup_map4);
> + }
> +
> + if (*fixup_prog1) {
> + map_fds[4] = create_prog_array(4, 0);
> + do {
> + prog[*fixup_prog1].imm = map_fds[4];
> + fixup_prog1++;
> + } while (*fixup_prog1);
> + }
> +
> + if (*fixup_prog2) {
> + map_fds[5] = create_prog_array(8, 7);
> do {
> - prog[*fixup_prog].imm = map_fds[2];
> - fixup_prog++;
> - } while (*fixup_prog);
> + prog[*fixup_prog2].imm = map_fds[5];
> + fixup_prog2++;
> + } while (*fixup_prog2);
> }
>
> if (*fixup_map_in_map) {
> - map_fds[3] = create_map_in_map();
> + map_fds[6] = create_map_in_map();
> do {
> - prog[*fixup_map_in_map].imm = map_fds[3];
> + prog[*fixup_map_in_map].imm = map_fds[6];
> fixup_map_in_map++;
> } while (*fixup_map_in_map);
> }
> --
> 2.9.5
>
next prev parent reply other threads:[~2018-05-29 18:01 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 [this message]
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 ` [PATCH bpf-next 09/11] bpf: fix context access in tracing progs on 32 bit archs Daniel Borkmann
2018-05-30 16:46 ` 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='CAPhsuW6Oo804YHfformbyNEV=CgFa5sSC0kOOarcHcHG9ysDXw@mail.gmail.com' \
--to=liu.song.a23@gmail.com \
--cc=ast@kernel.org \
--cc=daniel@iogearbox.net \
--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.