| From foo@baz Wed Apr 17 20:59:12 CEST 2019 |
| From: Balbir Singh <sblbir@amzn.com> |
| Date: Wed, 3 Apr 2019 18:39:09 +0000 |
| Subject: bpf: restrict stack pointer arithmetic for unprivileged |
| To: <gregkh@linuxfoundation.org> |
| Cc: <stable@kernel.org>, <daniel@iogearbox.net>, <jannh@google.com>, <sblbir@amazon.com>, Alexei Starovoitov <ast@kernel.org>, Balbir Singh <sblbir@amzn.com> |
| Message-ID: <20190403183917.13749-10-sblbir@amzn.com> |
| |
| From: Daniel Borkmann <daniel@iogearbox.net> |
| |
| commit e4298d25830a866cc0f427d4bccb858e76715859 upstream. |
| |
| Restrict stack pointer arithmetic for unprivileged users in that |
| arithmetic itself must not go out of bounds as opposed to the actual |
| access later on. Therefore after each adjust_ptr_min_max_vals() with |
| a stack pointer as a destination we simulate a check_stack_access() |
| of 1 byte on the destination and once that fails the program is |
| rejected for unprivileged program loads. This is analog to map |
| value pointer arithmetic and needed for masking later on. |
| |
| |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| [backported to 4.14 sblbir] |
| Signed-off-by: Balbir Singh <sblbir@amzn.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/bpf/verifier.c | 62 +++++++++++++++++++++++++++++++++----------------- |
| 1 file changed, 41 insertions(+), 21 deletions(-) |
| |
| --- a/kernel/bpf/verifier.c |
| +++ b/kernel/bpf/verifier.c |
| @@ -940,6 +940,31 @@ static int check_stack_read(struct bpf_v |
| } |
| } |
| |
| +static int check_stack_access(struct bpf_verifier_env *env, |
| + const struct bpf_reg_state *reg, |
| + int off, int size) |
| +{ |
| + /* Stack accesses must be at a fixed offset, so that we |
| + * can determine what type of data were returned. See |
| + * check_stack_read(). |
| + */ |
| + if (!tnum_is_const(reg->var_off)) { |
| + char tn_buf[48]; |
| + |
| + tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off); |
| + verbose("variable stack access var_off=%s off=%d size=%d", |
| + tn_buf, off, size); |
| + return -EACCES; |
| + } |
| + |
| + if (off >= 0 || off < -MAX_BPF_STACK) { |
| + verbose("invalid stack off=%d size=%d\n", off, size); |
| + return -EACCES; |
| + } |
| + |
| + return 0; |
| +} |
| + |
| /* check read/write into map element returned by bpf_map_lookup_elem() */ |
| static int __check_map_access(struct bpf_verifier_env *env, u32 regno, int off, |
| int size) |
| @@ -1322,23 +1347,10 @@ static int check_mem_access(struct bpf_v |
| } |
| |
| } else if (reg->type == PTR_TO_STACK) { |
| - /* stack accesses must be at a fixed offset, so that we can |
| - * determine what type of data were returned. |
| - * See check_stack_read(). |
| - */ |
| - if (!tnum_is_const(reg->var_off)) { |
| - char tn_buf[48]; |
| - |
| - tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off); |
| - verbose("variable stack access var_off=%s off=%d size=%d", |
| - tn_buf, off, size); |
| - return -EACCES; |
| - } |
| off += reg->var_off.value; |
| - if (off >= 0 || off < -MAX_BPF_STACK) { |
| - verbose("invalid stack off=%d size=%d\n", off, size); |
| - return -EACCES; |
| - } |
| + err = check_stack_access(env, reg, off, size); |
| + if (err) |
| + return err; |
| |
| if (env->prog->aux->stack_depth < -off) |
| env->prog->aux->stack_depth = -off; |
| @@ -2195,11 +2207,19 @@ static int adjust_ptr_min_max_vals(struc |
| /* For unprivileged we require that resulting offset must be in bounds |
| * in order to be able to sanitize access later on. |
| */ |
| - if (!env->allow_ptr_leaks && dst_reg->type == PTR_TO_MAP_VALUE && |
| - check_map_access(env, dst, dst_reg->off, 1, false)) { |
| - verbose(env, "R%d pointer arithmetic of map value goes out of range, prohibited for !root\n", |
| - dst); |
| - return -EACCES; |
| + if (!env->allow_ptr_leaks) { |
| + if (dst_reg->type == PTR_TO_MAP_VALUE && |
| + check_map_access(env, dst, dst_reg->off, 1)) { |
| + verbose("R%d pointer arithmetic of map value goes out of range, " |
| + "prohibited for !root\n", dst); |
| + return -EACCES; |
| + } else if (dst_reg->type == PTR_TO_STACK && |
| + check_stack_access(env, dst_reg, dst_reg->off + |
| + dst_reg->var_off.value, 1)) { |
| + verbose("R%d stack pointer arithmetic goes out of range, " |
| + "prohibited for !root\n", dst); |
| + return -EACCES; |
| + } |
| } |
| |
| return 0; |