* [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field
@ 2019-03-04 20:08 Daniel Borkmann
2019-03-04 23:01 ` Song Liu
2019-03-07 16:50 ` Alexei Starovoitov
0 siblings, 2 replies; 3+ messages in thread
From: Daniel Borkmann @ 2019-03-04 20:08 UTC (permalink / raw)
To: ast; +Cc: bpf, netdev, Daniel Borkmann
Non-zero imm value in the second part of the ldimm64 instruction for
BPF_PSEUDO_MAP_FD is invalid, and thus must be rejected. The map fd
only ever sits in the first instructions' imm field. None of the BPF
loaders known to us are using it, so risk of regression is minimal.
For clarity and consistency, the few insn->{src_reg,imm} occurrences
are rewritten into insn[0].{src_reg,imm}. Add a test case to the BPF
selftest suite as well.
Fixes: 0246e64d9a5f ("bpf: handle pseudo BPF_LD_IMM64 insn")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
[ Needs to wait until bpf tree has everything fast-forwarded from
Linus' tree. ]
kernel/bpf/verifier.c | 10 +++++-----
tools/testing/selftests/bpf/verifier/ld_imm64.c | 15 ++++++++++++++-
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e4edd7..c8d2a94 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6678,17 +6678,17 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
/* valid generic load 64-bit imm */
goto next_insn;
- if (insn->src_reg != BPF_PSEUDO_MAP_FD) {
- verbose(env,
- "unrecognized bpf_ld_imm64 insn\n");
+ if (insn[0].src_reg != BPF_PSEUDO_MAP_FD ||
+ insn[1].imm != 0) {
+ verbose(env, "unrecognized bpf_ld_imm64 insn\n");
return -EINVAL;
}
- f = fdget(insn->imm);
+ f = fdget(insn[0].imm);
map = __bpf_map_get(f);
if (IS_ERR(map)) {
verbose(env, "fd %d is not pointing to valid bpf_map\n",
- insn->imm);
+ insn[0].imm);
return PTR_ERR(map);
}
diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
index 28b8c80..3856dba 100644
--- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
+++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
@@ -122,7 +122,7 @@
.insns = {
BPF_MOV64_IMM(BPF_REG_1, 0),
BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
- BPF_RAW_INSN(0, 0, 0, 0, 1),
+ BPF_RAW_INSN(0, 0, 0, 0, 0),
BPF_EXIT_INSN(),
},
.errstr = "not pointing to valid bpf_map",
@@ -139,3 +139,16 @@
.errstr = "invalid bpf_ld_imm64 insn",
.result = REJECT,
},
+{
+ "test14 ld_imm64: reject 2nd imm != 0",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, BPF_REG_1,
+ BPF_PSEUDO_MAP_FD, 0, 0),
+ BPF_RAW_INSN(0, 0, 0, 0, 0xfefefe),
+ BPF_EXIT_INSN(),
+ },
+ .fixup_map_hash_48b = { 1 },
+ .errstr = "unrecognized bpf_ld_imm64 insn",
+ .result = REJECT,
+},
--
2.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field
2019-03-04 20:08 [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field Daniel Borkmann
@ 2019-03-04 23:01 ` Song Liu
2019-03-07 16:50 ` Alexei Starovoitov
1 sibling, 0 replies; 3+ messages in thread
From: Song Liu @ 2019-03-04 23:01 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, bpf, Networking
On Mon, Mar 4, 2019 at 12:09 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Non-zero imm value in the second part of the ldimm64 instruction for
> BPF_PSEUDO_MAP_FD is invalid, and thus must be rejected. The map fd
> only ever sits in the first instructions' imm field. None of the BPF
> loaders known to us are using it, so risk of regression is minimal.
> For clarity and consistency, the few insn->{src_reg,imm} occurrences
> are rewritten into insn[0].{src_reg,imm}. Add a test case to the BPF
> selftest suite as well.
>
> Fixes: 0246e64d9a5f ("bpf: handle pseudo BPF_LD_IMM64 insn")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> [ Needs to wait until bpf tree has everything fast-forwarded from
> Linus' tree. ]
>
> kernel/bpf/verifier.c | 10 +++++-----
> tools/testing/selftests/bpf/verifier/ld_imm64.c | 15 ++++++++++++++-
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0e4edd7..c8d2a94 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6678,17 +6678,17 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
> /* valid generic load 64-bit imm */
> goto next_insn;
>
> - if (insn->src_reg != BPF_PSEUDO_MAP_FD) {
> - verbose(env,
> - "unrecognized bpf_ld_imm64 insn\n");
> + if (insn[0].src_reg != BPF_PSEUDO_MAP_FD ||
> + insn[1].imm != 0) {
> + verbose(env, "unrecognized bpf_ld_imm64 insn\n");
> return -EINVAL;
> }
>
> - f = fdget(insn->imm);
> + f = fdget(insn[0].imm);
> map = __bpf_map_get(f);
> if (IS_ERR(map)) {
> verbose(env, "fd %d is not pointing to valid bpf_map\n",
> - insn->imm);
> + insn[0].imm);
> return PTR_ERR(map);
> }
>
> diff --git a/tools/testing/selftests/bpf/verifier/ld_imm64.c b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> index 28b8c80..3856dba 100644
> --- a/tools/testing/selftests/bpf/verifier/ld_imm64.c
> +++ b/tools/testing/selftests/bpf/verifier/ld_imm64.c
> @@ -122,7 +122,7 @@
> .insns = {
> BPF_MOV64_IMM(BPF_REG_1, 0),
> BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, 0, BPF_REG_1, 0, 1),
> - BPF_RAW_INSN(0, 0, 0, 0, 1),
> + BPF_RAW_INSN(0, 0, 0, 0, 0),
> BPF_EXIT_INSN(),
> },
> .errstr = "not pointing to valid bpf_map",
> @@ -139,3 +139,16 @@
> .errstr = "invalid bpf_ld_imm64 insn",
> .result = REJECT,
> },
> +{
> + "test14 ld_imm64: reject 2nd imm != 0",
> + .insns = {
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_RAW_INSN(BPF_LD | BPF_IMM | BPF_DW, BPF_REG_1,
> + BPF_PSEUDO_MAP_FD, 0, 0),
> + BPF_RAW_INSN(0, 0, 0, 0, 0xfefefe),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map_hash_48b = { 1 },
> + .errstr = "unrecognized bpf_ld_imm64 insn",
> + .result = REJECT,
> +},
> --
> 2.9.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field
2019-03-04 20:08 [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field Daniel Borkmann
2019-03-04 23:01 ` Song Liu
@ 2019-03-07 16:50 ` Alexei Starovoitov
1 sibling, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2019-03-07 16:50 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, bpf, netdev
On Mon, Mar 04, 2019 at 09:08:53PM +0100, Daniel Borkmann wrote:
> Non-zero imm value in the second part of the ldimm64 instruction for
> BPF_PSEUDO_MAP_FD is invalid, and thus must be rejected. The map fd
> only ever sits in the first instructions' imm field. None of the BPF
> loaders known to us are using it, so risk of regression is minimal.
> For clarity and consistency, the few insn->{src_reg,imm} occurrences
> are rewritten into insn[0].{src_reg,imm}. Add a test case to the BPF
> selftest suite as well.
>
> Fixes: 0246e64d9a5f ("bpf: handle pseudo BPF_LD_IMM64 insn")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied, Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-07 16:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 20:08 [PATCH bpf] bpf: fix replace_map_fd_with_map_ptr's ldimm64 second imm field Daniel Borkmann
2019-03-04 23:01 ` Song Liu
2019-03-07 16:50 ` Alexei Starovoitov
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).