| From e6cba8acf0f7f47cc1b8c5884dac40b451de9c58 Mon Sep 17 00:00:00 2001 |
| From: Daniel Borkmann <daniel@iogearbox.net> |
| Date: Mon, 28 Jan 2019 21:28:27 +0100 |
| Subject: bpf: fix check_map_access smin_value test when pointer contains |
| offset |
| |
| [ commit b7137c4eab85c1cf3d46acdde90ce1163b28c873 upstream ] |
| |
| In check_map_access() we probe actual bounds through __check_map_access() |
| with offset of reg->smin_value + off for lower bound and offset of |
| reg->umax_value + off for the upper bound. However, even though the |
| reg->smin_value could have a negative value, the final result of the |
| sum with off could be positive when pointer arithmetic with known and |
| unknown scalars is combined. In this case we reject the program with |
| an error such as "R<x> min value is negative, either use unsigned index |
| or do a if (index >=0) check." even though the access itself would be |
| fine. Therefore extend the check to probe whether the actual resulting |
| reg->smin_value + off is less than zero. |
| |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/bpf/verifier.c | 6 +++++- |
| 1 file changed, 5 insertions(+), 1 deletion(-) |
| |
| diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c |
| index fbaa3b9e1d71..f9d5aea4891d 100644 |
| --- a/kernel/bpf/verifier.c |
| +++ b/kernel/bpf/verifier.c |
| @@ -1294,13 +1294,17 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno, |
| */ |
| if (env->log.level) |
| print_verifier_state(env, state); |
| + |
| /* The minimum value is only important with signed |
| * comparisons where we can't assume the floor of a |
| * value is 0. If we are using signed variables for our |
| * index'es we need to make sure that whatever we use |
| * will have a set floor within our range. |
| */ |
| - if (reg->smin_value < 0) { |
| + if (reg->smin_value < 0 && |
| + (reg->smin_value == S64_MIN || |
| + (off + reg->smin_value != (s64)(s32)(off + reg->smin_value)) || |
| + reg->smin_value + off < 0)) { |
| verbose(env, "R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n", |
| regno); |
| return -EACCES; |
| -- |
| 2.19.1 |
| |