* [PATCH 0/3] bpf: Test defence against SSB exploitation
@ 2018-11-23 18:33 Ben Hutchings
2018-11-23 18:34 ` [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled Ben Hutchings
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ben Hutchings @ 2018-11-23 18:33 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
This series adds log messages for all patching done by the verifier,
and a test case to verify that the patch to defend against SSB
exploitation is applied where needed.
Ben.
Ben Hutchings (3):
bpf/verifier: Log instruction patching when verbose logging is enabled
selftests/bpf: Add the ability to test for a log message on success
selftests/bpf: Add test case for defence against SSB exploitation
kernel/bpf/verifier.c | 13 +++++++++++++
tools/testing/selftests/bpf/test_verifier.c | 26 +++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled
2018-11-23 18:33 [PATCH 0/3] bpf: Test defence against SSB exploitation Ben Hutchings
@ 2018-11-23 18:34 ` Ben Hutchings
2018-11-23 20:10 ` Daniel Borkmann
2018-11-23 18:35 ` [PATCH 2/3] selftests/bpf: Add the ability to test for a log message on success Ben Hutchings
2018-11-23 18:35 ` [PATCH 3/3] selftests/bpf: Add test case for defence against SSB exploitation Ben Hutchings
2 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2018-11-23 18:34 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
User-space does not have access to the patched eBPF code, but we
need to be able to test that patches are being applied. Therefore
log distinct messages for each case that requires patching.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
kernel/bpf/verifier.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4ce049cd30a3..ea4bc796e545 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5844,6 +5844,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
verbose(env, "bpf verifier is misconfigured\n");
return -EINVAL;
} else if (cnt) {
+ verbose(env, "patching in prologue\n");
new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
if (!new_prog)
return -ENOMEM;
@@ -5892,6 +5893,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
};
cnt = ARRAY_SIZE(patch);
+ verbose(env,
+ "patching in sanitization against SSB at %d\n",
+ i + delta);
new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt);
if (!new_prog)
return -ENOMEM;
@@ -5973,6 +5977,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
}
}
+ verbose(env, "patching explicit ctx access at %d\n", i + delta);
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
if (!new_prog)
return -ENOMEM;
@@ -6225,6 +6230,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
cnt = ARRAY_SIZE(mask_and_mod) - (is64 ? 1 : 0);
}
+ verbose(env, "patching in divide-by-zero check at %d\n",
+ i + delta);
new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
if (!new_prog)
return -ENOMEM;
@@ -6244,6 +6251,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
return -EINVAL;
}
+ verbose(env, "patching implicit ctx access at %d\n",
+ i + delta);
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
if (!new_prog)
return -ENOMEM;
@@ -6307,6 +6316,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
map)->index_mask);
insn_buf[2] = *insn;
cnt = 3;
+ verbose(env, "patching in tail-call bounds check at %d",
+ i + delta);
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
if (!new_prog)
return -ENOMEM;
@@ -6342,6 +6353,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
return -EINVAL;
}
+ verbose(env, "patching in map lookup at %d",
+ i + delta);
new_prog = bpf_patch_insn_data(env, i + delta,
insn_buf, cnt);
if (!new_prog)
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] selftests/bpf: Add the ability to test for a log message on success
2018-11-23 18:33 [PATCH 0/3] bpf: Test defence against SSB exploitation Ben Hutchings
2018-11-23 18:34 ` [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled Ben Hutchings
@ 2018-11-23 18:35 ` Ben Hutchings
2018-11-23 18:35 ` [PATCH 3/3] selftests/bpf: Add test case for defence against SSB exploitation Ben Hutchings
2 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2018-11-23 18:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
This is needed to test that code is being patched when it should be.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
tools/testing/selftests/bpf/test_verifier.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 0f3f97a401c9..e71b7f2e5f17 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -76,6 +76,8 @@ struct bpf_test {
int fixup_percpu_cgroup_storage[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
+ const char *infostr;
+ const char *infostr_unpriv;
uint32_t retval, retval_unpriv;
enum {
UNDEF,
@@ -14232,7 +14234,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int prog_len, prog_type = test->prog_type;
struct bpf_insn *prog = test->insns;
int map_fds[MAX_NR_MAPS];
- const char *expected_err;
+ const char *expected_err, *expected_info;
uint32_t expected_val;
uint32_t retval;
int i, err;
@@ -14253,6 +14255,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
test->result_unpriv : test->result;
expected_err = unpriv && test->errstr_unpriv ?
test->errstr_unpriv : test->errstr;
+ expected_info = unpriv && test->infostr_unpriv ?
+ test->infostr_unpriv : test->infostr;
expected_val = unpriv && test->retval_unpriv ?
test->retval_unpriv : test->retval;
@@ -14272,6 +14276,11 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
strerror(errno));
goto fail_log;
}
+ if (expected_info && !strstr(bpf_vlog, expected_info)) {
+ printf("FAIL\nMissing expected info message!\n\tEXP: %s\n\tRES: %s\n",
+ expected_info, bpf_vlog);
+ goto fail_log;
+ }
} else {
if (fd_prog >= 0) {
printf("FAIL\nUnexpected success to load!\n");
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] selftests/bpf: Add test case for defence against SSB exploitation
2018-11-23 18:33 [PATCH 0/3] bpf: Test defence against SSB exploitation Ben Hutchings
2018-11-23 18:34 ` [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled Ben Hutchings
2018-11-23 18:35 ` [PATCH 2/3] selftests/bpf: Add the ability to test for a log message on success Ben Hutchings
@ 2018-11-23 18:35 ` Ben Hutchings
2 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2018-11-23 18:35 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
Test that the defence added by commit af86ca4e3088 "bpf: Prevent
memory disambiguation attack" is actually being applied.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
tools/testing/selftests/bpf/test_verifier.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index e71b7f2e5f17..ca21a63541b0 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -13927,6 +13927,21 @@ static struct bpf_test tests[] = {
.result = ACCEPT,
},
{
+ "reference tracking: defend against SSB exploitation",
+ .insns = {
+ BPF_MOV32_IMM(BPF_REG_2, 1),
+ /* stack[-1] = (integer) 1 */
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8),
+ /* stack[-1] = (pointer) context */
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_1, -8),
+ BPF_MOV32_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .infostr_unpriv = "patching in sanitization against SSB at 2",
+ .result_unpriv = ACCEPT,
+ .result = ACCEPT,
+ },
+ {
"calls: ctx read at start of subprog",
.insns = {
BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled
2018-11-23 18:34 ` [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled Ben Hutchings
@ 2018-11-23 20:10 ` Daniel Borkmann
2018-11-29 15:55 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2018-11-23 20:10 UTC (permalink / raw)
To: Ben Hutchings, Alexei Starovoitov; +Cc: netdev, linux-kernel
On 11/23/2018 07:34 PM, Ben Hutchings wrote:
> User-space does not have access to the patched eBPF code, but we
> need to be able to test that patches are being applied. Therefore
> log distinct messages for each case that requires patching.
Thanks for the patches, Ben! Above is actually not the case, e.g. privileged
admin can use something like 'bpftool prog dump xlated id <id>' and then the
BPF insns are dumped to user space for the program /after/ the verification,
so the rewrites can then be seen. test_verifier temporarily drops caps to
load and run the unprivileged cases, but we can extend the test suite to
retrieve and check the final insns after that happened. I think this would be
a nice extension to the test suite for cases like these and would also provide
better insight than verbose() statement saying that something has been
patched (but not the actual result of it).
Thanks,
Daniel
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
> kernel/bpf/verifier.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 4ce049cd30a3..ea4bc796e545 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5844,6 +5844,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> verbose(env, "bpf verifier is misconfigured\n");
> return -EINVAL;
> } else if (cnt) {
> + verbose(env, "patching in prologue\n");
> new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -5892,6 +5893,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> };
>
> cnt = ARRAY_SIZE(patch);
> + verbose(env,
> + "patching in sanitization against SSB at %d\n",
> + i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta, patch, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -5973,6 +5977,7 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> }
> }
>
> + verbose(env, "patching explicit ctx access at %d\n", i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -6225,6 +6230,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> cnt = ARRAY_SIZE(mask_and_mod) - (is64 ? 1 : 0);
> }
>
> + verbose(env, "patching in divide-by-zero check at %d\n",
> + i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta, patchlet, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -6244,6 +6251,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> return -EINVAL;
> }
>
> + verbose(env, "patching implicit ctx access at %d\n",
> + i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -6307,6 +6316,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> map)->index_mask);
> insn_buf[2] = *insn;
> cnt = 3;
> + verbose(env, "patching in tail-call bounds check at %d",
> + i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
> if (!new_prog)
> return -ENOMEM;
> @@ -6342,6 +6353,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> return -EINVAL;
> }
>
> + verbose(env, "patching in map lookup at %d",
> + i + delta);
> new_prog = bpf_patch_insn_data(env, i + delta,
> insn_buf, cnt);
> if (!new_prog)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled
2018-11-23 20:10 ` Daniel Borkmann
@ 2018-11-29 15:55 ` Ben Hutchings
0 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2018-11-29 15:55 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev, linux-kernel
On Fri, 2018-11-23 at 21:10 +0100, Daniel Borkmann wrote:
> On 11/23/2018 07:34 PM, Ben Hutchings wrote:
> > User-space does not have access to the patched eBPF code, but we
> > need to be able to test that patches are being applied. Therefore
> > log distinct messages for each case that requires patching.
>
> Thanks for the patches, Ben! Above is actually not the case, e.g. privileged
> admin can use something like 'bpftool prog dump xlated id <id>' and then the
> BPF insns are dumped to user space for the program /after/ the verification,
> so the rewrites can then be seen.
Oh that's good.
> test_verifier temporarily drops caps to
> load and run the unprivileged cases, but we can extend the test suite to
> retrieve and check the final insns after that happened. I think this would be
> a nice extension to the test suite for cases like these and would also provide
> better insight than verbose() statement saying that something has been
> patched (but not the actual result of it).
Agreed; I'll look into doing this instead.
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-29 15:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-23 18:33 [PATCH 0/3] bpf: Test defence against SSB exploitation Ben Hutchings
2018-11-23 18:34 ` [PATCH 1/3] bpf/verifier: Log instruction patching when verbose logging is enabled Ben Hutchings
2018-11-23 20:10 ` Daniel Borkmann
2018-11-29 15:55 ` Ben Hutchings
2018-11-23 18:35 ` [PATCH 2/3] selftests/bpf: Add the ability to test for a log message on success Ben Hutchings
2018-11-23 18:35 ` [PATCH 3/3] selftests/bpf: Add test case for defence against SSB exploitation Ben Hutchings
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).