| Message ID | 20210419235641.5442-1-samjonas@amazon.com |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: Fix backport of "bpf: restrict unknown scalars of mixed signed bounds for unprivileged" | expand |
On 4/19/21 4:56 PM, Samuel Mendoza-Jonas wrote: > The 4.14 backport of 9d7eceede ("bpf: restrict unknown scalars of mixed > signed bounds for unprivileged") adds the PTR_TO_MAP_VALUE check to the > wrong location in adjust_ptr_min_max_vals(), most likely because 4.14 > doesn't include the commit that updates the if-statement to a > switch-statement (aad2eeaf4 "bpf: Simplify ptr_min_max_vals adjustment"). > > Move the check to the proper location in adjust_ptr_min_max_vals(). > > Fixes: 17efa65350c5a ("bpf: restrict unknown scalars of mixed signed bounds for unprivileged") > Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com> > Reviewed-by: Frank van der Linden <fllinden@amazon.com> > Reviewed-by: Ethan Chen <yishache@amazon.com> Just to be clear, the patch is for 4.14 stable branch. Acked-by: Yonghong Song <yhs@fb.com>
On Mon, Apr 19, 2021 at 04:56:41PM -0700, Samuel Mendoza-Jonas wrote: > The 4.14 backport of 9d7eceede ("bpf: restrict unknown scalars of mixed > signed bounds for unprivileged") adds the PTR_TO_MAP_VALUE check to the > wrong location in adjust_ptr_min_max_vals(), most likely because 4.14 > doesn't include the commit that updates the if-statement to a > switch-statement (aad2eeaf4 "bpf: Simplify ptr_min_max_vals adjustment"). > > Move the check to the proper location in adjust_ptr_min_max_vals(). > > Fixes: 17efa65350c5a ("bpf: restrict unknown scalars of mixed signed bounds for unprivileged") > Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com> > Reviewed-by: Frank van der Linden <fllinden@amazon.com> > Reviewed-by: Ethan Chen <yishache@amazon.com> > --- Thanks for catching it :) Reviewed-by: Balbir Singh <bsingharora@gmail.com>
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0c3a9302be93..9e9b7c076bcb 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -2204,6 +2204,13 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, dst); return -EACCES; } + if (ptr_reg->type == PTR_TO_MAP_VALUE) { + if (!env->allow_ptr_leaks && !known && (smin_val < 0) != (smax_val < 0)) { + verbose("R%d has unknown scalar with mixed signed bounds, pointer arithmetic with it prohibited for !root\n", + off_reg == dst_reg ? dst : src); + return -EACCES; + } + } /* In case of 'scalar += pointer', dst_reg inherits pointer type and id. * The id may be overwritten later if we create a new variable offset. @@ -2349,13 +2356,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, verbose("R%d bitwise operator %s on pointer prohibited\n", dst, bpf_alu_string[opcode >> 4]); return -EACCES; - case PTR_TO_MAP_VALUE: - if (!env->allow_ptr_leaks && !known && (smin_val < 0) != (smax_val < 0)) { - verbose("R%d has unknown scalar with mixed signed bounds, pointer arithmetic with it prohibited for !root\n", - off_reg == dst_reg ? dst : src); - return -EACCES; - } - /* fall-through */ default: /* other operators (e.g. MUL,LSH) produce non-pointer results */ if (!env->allow_ptr_leaks)
The 4.14 backport of 9d7eceede ("bpf: restrict unknown scalars of mixed signed bounds for unprivileged") adds the PTR_TO_MAP_VALUE check to the wrong location in adjust_ptr_min_max_vals(), most likely because 4.14 doesn't include the commit that updates the if-statement to a switch-statement (aad2eeaf4 "bpf: Simplify ptr_min_max_vals adjustment"). Move the check to the proper location in adjust_ptr_min_max_vals(). Fixes: 17efa65350c5a ("bpf: restrict unknown scalars of mixed signed bounds for unprivileged") Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com> Reviewed-by: Frank van der Linden <fllinden@amazon.com> Reviewed-by: Ethan Chen <yishache@amazon.com> --- kernel/bpf/verifier.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)