From 70a31745f20d8360d2c14f1d0cd8993ac2d938e1 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 30 Apr 2021 15:38:25 +0200 Subject: [PATCH] 5.4-stable patches added patches: bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch bpf-improve-verifier-error-messages-for-users.patch bpf-move-off_reg-into-sanitize_ptr_alu.patch bpf-move-sanitize_val_alu-out-of-op-switch.patch bpf-refactor-and-streamline-bounds-check-into-helper.patch bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch bpf-tighten-speculative-pointer-arithmetic-mask.patch bpf-update-selftests-to-reflect-new-error-states.patch --- ...no-mixed-signed-bounds-for-all-types.patch | 87 ++++++ ...ve-verifier-error-messages-for-users.patch | 184 ++++++++++++ ...f-move-off_reg-into-sanitize_ptr_alu.patch | 58 ++++ ...ve-sanitize_val_alu-out-of-op-switch.patch | 67 +++++ ...-streamline-bounds-check-into-helper.patch | 95 ++++++ ...-alu_limit-and-add-common-error-path.patch | 74 +++++ ...-speculative-pointer-arithmetic-mask.patch | 201 +++++++++++++ ...elftests-to-reflect-new-error-states.patch | 274 ++++++++++++++++++ queue-5.4/series | 8 + 9 files changed, 1048 insertions(+) create mode 100644 queue-5.4/bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch create mode 100644 queue-5.4/bpf-improve-verifier-error-messages-for-users.patch create mode 100644 queue-5.4/bpf-move-off_reg-into-sanitize_ptr_alu.patch create mode 100644 queue-5.4/bpf-move-sanitize_val_alu-out-of-op-switch.patch create mode 100644 queue-5.4/bpf-refactor-and-streamline-bounds-check-into-helper.patch create mode 100644 queue-5.4/bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch create mode 100644 queue-5.4/bpf-tighten-speculative-pointer-arithmetic-mask.patch create mode 100644 queue-5.4/bpf-update-selftests-to-reflect-new-error-states.patch create mode 100644 queue-5.4/series diff --git a/queue-5.4/bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch b/queue-5.4/bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch new file mode 100644 index 00000000000..820a5b9fd83 --- /dev/null +++ b/queue-5.4/bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch @@ -0,0 +1,87 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:33 +0000 +Subject: bpf: Ensure off_reg has no mixed signed bounds for all types +To: +Cc: +Message-ID: <20210429220839.15667-3-fllinden@amazon.com> + +From: Daniel Borkmann + +commit 24c109bb1537c12c02aeed2d51a347b4d6a9b76e upstream. + +The mixed signed bounds check really belongs into retrieve_ptr_limit() +instead of outside of it in adjust_ptr_min_max_vals(). The reason is +that this check is not tied to PTR_TO_MAP_VALUE only, but to all pointer +types that we handle in retrieve_ptr_limit() and given errors from the latter +propagate back to adjust_ptr_min_max_vals() and lead to rejection of the +program, it's a better place to reside to avoid anything slipping through +for future types. The reason why we must reject such off_reg is that we +otherwise would not be able to derive a mask, see details in 9d7eceede769 +("bpf: restrict unknown scalars of mixed signed bounds for unprivileged"). + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +[fllinden@amazon.com: backport to 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 19 +++++++++---------- + 1 file changed, 9 insertions(+), 10 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4264,12 +4264,18 @@ static struct bpf_insn_aux_data *cur_aux + } + + static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg, +- u32 *ptr_limit, u8 opcode, bool off_is_neg) ++ const struct bpf_reg_state *off_reg, ++ u32 *ptr_limit, u8 opcode) + { ++ bool off_is_neg = off_reg->smin_value < 0; + bool mask_to_left = (opcode == BPF_ADD && off_is_neg) || + (opcode == BPF_SUB && !off_is_neg); + u32 off, max; + ++ if (!tnum_is_const(off_reg->var_off) && ++ (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) ++ return -EACCES; ++ + switch (ptr_reg->type) { + case PTR_TO_STACK: + /* Offset 0 is out-of-bounds, but acceptable start for the +@@ -4363,7 +4369,7 @@ static int sanitize_ptr_alu(struct bpf_v + alu_state |= ptr_is_dst_reg ? + BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST; + +- err = retrieve_ptr_limit(ptr_reg, &alu_limit, opcode, off_is_neg); ++ err = retrieve_ptr_limit(ptr_reg, off_reg, &alu_limit, opcode); + if (err < 0) + return err; + +@@ -4408,8 +4414,8 @@ static int adjust_ptr_min_max_vals(struc + smin_ptr = ptr_reg->smin_value, smax_ptr = ptr_reg->smax_value; + u64 umin_val = off_reg->umin_value, umax_val = off_reg->umax_value, + umin_ptr = ptr_reg->umin_value, umax_ptr = ptr_reg->umax_value; +- u32 dst = insn->dst_reg, src = insn->src_reg; + u8 opcode = BPF_OP(insn->code); ++ u32 dst = insn->dst_reg; + int ret; + + dst_reg = ®s[dst]; +@@ -4452,13 +4458,6 @@ static int adjust_ptr_min_max_vals(struc + verbose(env, "R%d pointer arithmetic on %s prohibited\n", + dst, reg_type_str[ptr_reg->type]); + return -EACCES; +- case PTR_TO_MAP_VALUE: +- if (!env->allow_ptr_leaks && !known && (smin_val < 0) != (smax_val < 0)) { +- verbose(env, "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: + break; + } diff --git a/queue-5.4/bpf-improve-verifier-error-messages-for-users.patch b/queue-5.4/bpf-improve-verifier-error-messages-for-users.patch new file mode 100644 index 00000000000..5f0c5511b3e --- /dev/null +++ b/queue-5.4/bpf-improve-verifier-error-messages-for-users.patch @@ -0,0 +1,184 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:35 +0000 +Subject: bpf: Improve verifier error messages for users +To: +Cc: +Message-ID: <20210429220839.15667-5-fllinden@amazon.com> + +From: Daniel Borkmann + +commit a6aaece00a57fa6f22575364b3903dfbccf5345d upstream. + +Consolidate all error handling and provide more user-friendly error messages +from sanitize_ptr_alu() and sanitize_val_alu(). + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +[fllinden@amazon.com: backport to 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 84 ++++++++++++++++++++++++++++++++++++-------------- + 1 file changed, 62 insertions(+), 22 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4263,6 +4263,14 @@ static struct bpf_insn_aux_data *cur_aux + return &env->insn_aux_data[env->insn_idx]; + } + ++enum { ++ REASON_BOUNDS = -1, ++ REASON_TYPE = -2, ++ REASON_PATHS = -3, ++ REASON_LIMIT = -4, ++ REASON_STACK = -5, ++}; ++ + static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg, + const struct bpf_reg_state *off_reg, + u32 *alu_limit, u8 opcode) +@@ -4274,7 +4282,7 @@ static int retrieve_ptr_limit(const stru + + if (!tnum_is_const(off_reg->var_off) && + (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) +- return -EACCES; ++ return REASON_BOUNDS; + + switch (ptr_reg->type) { + case PTR_TO_STACK: +@@ -4301,11 +4309,11 @@ static int retrieve_ptr_limit(const stru + } + break; + default: +- return -EINVAL; ++ return REASON_TYPE; + } + + if (ptr_limit >= max) +- return -ERANGE; ++ return REASON_LIMIT; + *alu_limit = ptr_limit; + return 0; + } +@@ -4325,7 +4333,7 @@ static int update_alu_sanitation_state(s + if (aux->alu_state && + (aux->alu_state != alu_state || + aux->alu_limit != alu_limit)) +- return -EACCES; ++ return REASON_PATHS; + + /* Corresponding fixup done in fixup_bpf_calls(). */ + aux->alu_state = alu_state; +@@ -4398,7 +4406,46 @@ do_sim: + ret = push_stack(env, env->insn_idx + 1, env->insn_idx, true); + if (!ptr_is_dst_reg && ret) + *dst_reg = tmp; +- return !ret ? -EFAULT : 0; ++ return !ret ? REASON_STACK : 0; ++} ++ ++static int sanitize_err(struct bpf_verifier_env *env, ++ const struct bpf_insn *insn, int reason, ++ const struct bpf_reg_state *off_reg, ++ const struct bpf_reg_state *dst_reg) ++{ ++ static const char *err = "pointer arithmetic with it prohibited for !root"; ++ const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub"; ++ u32 dst = insn->dst_reg, src = insn->src_reg; ++ ++ switch (reason) { ++ case REASON_BOUNDS: ++ verbose(env, "R%d has unknown scalar with mixed signed bounds, %s\n", ++ off_reg == dst_reg ? dst : src, err); ++ break; ++ case REASON_TYPE: ++ verbose(env, "R%d has pointer with unsupported alu operation, %s\n", ++ off_reg == dst_reg ? src : dst, err); ++ break; ++ case REASON_PATHS: ++ verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n", ++ dst, op, err); ++ break; ++ case REASON_LIMIT: ++ verbose(env, "R%d tried to %s beyond pointer bounds, %s\n", ++ dst, op, err); ++ break; ++ case REASON_STACK: ++ verbose(env, "R%d could not be pushed for speculative verification, %s\n", ++ dst, err); ++ break; ++ default: ++ verbose(env, "verifier internal error: unknown reason (%d)\n", ++ reason); ++ break; ++ } ++ ++ return -EACCES; + } + + /* Handles arithmetic on a pointer and a scalar: computes new min/max and var_off. +@@ -4480,10 +4527,9 @@ static int adjust_ptr_min_max_vals(struc + switch (opcode) { + case BPF_ADD: + ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); +- if (ret < 0) { +- verbose(env, "R%d tried to add from different maps, paths, or prohibited types\n", dst); +- return ret; +- } ++ if (ret < 0) ++ return sanitize_err(env, insn, ret, off_reg, dst_reg); ++ + /* We can take a fixed offset as long as it doesn't overflow + * the s32 'off' field + */ +@@ -4535,10 +4581,9 @@ static int adjust_ptr_min_max_vals(struc + break; + case BPF_SUB: + ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); +- if (ret < 0) { +- verbose(env, "R%d tried to sub from different maps, paths, or prohibited types\n", dst); +- return ret; +- } ++ if (ret < 0) ++ return sanitize_err(env, insn, ret, off_reg, dst_reg); ++ + if (dst_reg == off_reg) { + /* scalar -= pointer. Creates an unknown scalar */ + verbose(env, "R%d tried to subtract pointer from scalar\n", +@@ -4655,7 +4700,6 @@ static int adjust_scalar_min_max_vals(st + s64 smin_val, smax_val; + u64 umin_val, umax_val; + u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32; +- u32 dst = insn->dst_reg; + int ret; + + if (insn_bitness == 32) { +@@ -4692,10 +4736,8 @@ static int adjust_scalar_min_max_vals(st + switch (opcode) { + case BPF_ADD: + ret = sanitize_val_alu(env, insn); +- if (ret < 0) { +- verbose(env, "R%d tried to add from different pointers or scalars\n", dst); +- return ret; +- } ++ if (ret < 0) ++ return sanitize_err(env, insn, ret, NULL, NULL); + if (signed_add_overflows(dst_reg->smin_value, smin_val) || + signed_add_overflows(dst_reg->smax_value, smax_val)) { + dst_reg->smin_value = S64_MIN; +@@ -4716,10 +4758,8 @@ static int adjust_scalar_min_max_vals(st + break; + case BPF_SUB: + ret = sanitize_val_alu(env, insn); +- if (ret < 0) { +- verbose(env, "R%d tried to sub from different pointers or scalars\n", dst); +- return ret; +- } ++ if (ret < 0) ++ return sanitize_err(env, insn, ret, NULL, NULL); + if (signed_sub_overflows(dst_reg->smin_value, smax_val) || + signed_sub_overflows(dst_reg->smax_value, smin_val)) { + /* Overflow possible, we know nothing */ diff --git a/queue-5.4/bpf-move-off_reg-into-sanitize_ptr_alu.patch b/queue-5.4/bpf-move-off_reg-into-sanitize_ptr_alu.patch new file mode 100644 index 00000000000..9edb68e192c --- /dev/null +++ b/queue-5.4/bpf-move-off_reg-into-sanitize_ptr_alu.patch @@ -0,0 +1,58 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:32 +0000 +Subject: bpf: Move off_reg into sanitize_ptr_alu +To: +Cc: +Message-ID: <20210429220839.15667-2-fllinden@amazon.com> + +From: Daniel Borkmann + +commit 6f55b2f2a1178856c19bbce2f71449926e731914 upstream. + +Small refactor to drag off_reg into sanitize_ptr_alu(), so we later on can +use off_reg for generalizing some of the checks for all pointer types. + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 9 +++++---- + 1 file changed, 5 insertions(+), 4 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4336,11 +4336,12 @@ static int sanitize_val_alu(struct bpf_v + static int sanitize_ptr_alu(struct bpf_verifier_env *env, + struct bpf_insn *insn, + const struct bpf_reg_state *ptr_reg, +- struct bpf_reg_state *dst_reg, +- bool off_is_neg) ++ const struct bpf_reg_state *off_reg, ++ struct bpf_reg_state *dst_reg) + { + struct bpf_verifier_state *vstate = env->cur_state; + struct bpf_insn_aux_data *aux = cur_aux(env); ++ bool off_is_neg = off_reg->smin_value < 0; + bool ptr_is_dst_reg = ptr_reg == dst_reg; + u8 opcode = BPF_OP(insn->code); + u32 alu_state, alu_limit; +@@ -4474,7 +4475,7 @@ static int adjust_ptr_min_max_vals(struc + + switch (opcode) { + case BPF_ADD: +- ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0); ++ ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); + if (ret < 0) { + verbose(env, "R%d tried to add from different maps, paths, or prohibited types\n", dst); + return ret; +@@ -4529,7 +4530,7 @@ static int adjust_ptr_min_max_vals(struc + } + break; + case BPF_SUB: +- ret = sanitize_ptr_alu(env, insn, ptr_reg, dst_reg, smin_val < 0); ++ ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); + if (ret < 0) { + verbose(env, "R%d tried to sub from different maps, paths, or prohibited types\n", dst); + return ret; diff --git a/queue-5.4/bpf-move-sanitize_val_alu-out-of-op-switch.patch b/queue-5.4/bpf-move-sanitize_val_alu-out-of-op-switch.patch new file mode 100644 index 00000000000..976b60abeb7 --- /dev/null +++ b/queue-5.4/bpf-move-sanitize_val_alu-out-of-op-switch.patch @@ -0,0 +1,67 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:37 +0000 +Subject: bpf: Move sanitize_val_alu out of op switch +To: +Cc: +Message-ID: <20210429220839.15667-7-fllinden@amazon.com> + +From: Daniel Borkmann + +commit f528819334881fd622fdadeddb3f7edaed8b7c9b upstream. + +Add a small sanitize_needed() helper function and move sanitize_val_alu() +out of the main opcode switch. In upcoming work, we'll move sanitize_ptr_alu() +as well out of its opcode switch so this helps to streamline both. + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +[fllinden@amazon.com: backported to 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 15 ++++++++++----- + 1 file changed, 10 insertions(+), 5 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4352,6 +4352,11 @@ static int sanitize_val_alu(struct bpf_v + return update_alu_sanitation_state(aux, BPF_ALU_NON_POINTER, 0); + } + ++static bool sanitize_needed(u8 opcode) ++{ ++ return opcode == BPF_ADD || opcode == BPF_SUB; ++} ++ + static int sanitize_ptr_alu(struct bpf_verifier_env *env, + struct bpf_insn *insn, + const struct bpf_reg_state *ptr_reg, +@@ -4753,11 +4758,14 @@ static int adjust_scalar_min_max_vals(st + return 0; + } + +- switch (opcode) { +- case BPF_ADD: ++ if (sanitize_needed(opcode)) { + ret = sanitize_val_alu(env, insn); + if (ret < 0) + return sanitize_err(env, insn, ret, NULL, NULL); ++ } ++ ++ switch (opcode) { ++ case BPF_ADD: + if (signed_add_overflows(dst_reg->smin_value, smin_val) || + signed_add_overflows(dst_reg->smax_value, smax_val)) { + dst_reg->smin_value = S64_MIN; +@@ -4777,9 +4785,6 @@ static int adjust_scalar_min_max_vals(st + dst_reg->var_off = tnum_add(dst_reg->var_off, src_reg.var_off); + break; + case BPF_SUB: +- ret = sanitize_val_alu(env, insn); +- if (ret < 0) +- return sanitize_err(env, insn, ret, NULL, NULL); + if (signed_sub_overflows(dst_reg->smin_value, smax_val) || + signed_sub_overflows(dst_reg->smax_value, smin_val)) { + /* Overflow possible, we know nothing */ diff --git a/queue-5.4/bpf-refactor-and-streamline-bounds-check-into-helper.patch b/queue-5.4/bpf-refactor-and-streamline-bounds-check-into-helper.patch new file mode 100644 index 00000000000..fb6a30fc9ec --- /dev/null +++ b/queue-5.4/bpf-refactor-and-streamline-bounds-check-into-helper.patch @@ -0,0 +1,95 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:36 +0000 +Subject: bpf: Refactor and streamline bounds check into helper +To: +Cc: +Message-ID: <20210429220839.15667-6-fllinden@amazon.com> + +From: Daniel Borkmann + +commit 073815b756c51ba9d8384d924c5d1c03ca3d1ae4 upstream. + +Move the bounds check in adjust_ptr_min_max_vals() into a small helper named +sanitize_check_bounds() in order to simplify the former a bit. + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +[fllinden@amazon.com: backport to 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 54 ++++++++++++++++++++++++++++++++++---------------- + 1 file changed, 37 insertions(+), 17 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4448,6 +4448,41 @@ static int sanitize_err(struct bpf_verif + return -EACCES; + } + ++static int sanitize_check_bounds(struct bpf_verifier_env *env, ++ const struct bpf_insn *insn, ++ const struct bpf_reg_state *dst_reg) ++{ ++ u32 dst = insn->dst_reg; ++ ++ /* 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) ++ return 0; ++ ++ switch (dst_reg->type) { ++ case PTR_TO_STACK: ++ if (check_stack_access(env, dst_reg, dst_reg->off + ++ dst_reg->var_off.value, 1)) { ++ verbose(env, "R%d stack pointer arithmetic goes out of range, " ++ "prohibited for !root\n", dst); ++ return -EACCES; ++ } ++ break; ++ case PTR_TO_MAP_VALUE: ++ if (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; ++ } ++ break; ++ default: ++ break; ++ } ++ ++ return 0; ++} ++ + /* Handles arithmetic on a pointer and a scalar: computes new min/max and var_off. + * Caller should also handle BPF_MOV case separately. + * If we return -EACCES, caller may want to try again treating pointer as a +@@ -4664,23 +4699,8 @@ static int adjust_ptr_min_max_vals(struc + __reg_deduce_bounds(dst_reg); + __reg_bound_offset(dst_reg); + +- /* 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) { +- if (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; +- } else if (dst_reg->type == PTR_TO_STACK && +- check_stack_access(env, dst_reg, dst_reg->off + +- dst_reg->var_off.value, 1)) { +- verbose(env, "R%d stack pointer arithmetic goes out of range, " +- "prohibited for !root\n", dst); +- return -EACCES; +- } +- } ++ if (sanitize_check_bounds(env, insn, dst_reg) < 0) ++ return -EACCES; + + return 0; + } diff --git a/queue-5.4/bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch b/queue-5.4/bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch new file mode 100644 index 00000000000..ca44a06a955 --- /dev/null +++ b/queue-5.4/bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch @@ -0,0 +1,74 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:34 +0000 +Subject: bpf: Rework ptr_limit into alu_limit and add common error path +To: +Cc: +Message-ID: <20210429220839.15667-4-fllinden@amazon.com> + +From: Daniel Borkmann + +commit b658bbb844e28f1862867f37e8ca11a8e2aa94a3 upstream. + +Small refactor with no semantic changes in order to consolidate the max +ptr_limit boundary check. + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 21 +++++++++++++-------- + 1 file changed, 13 insertions(+), 8 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4265,12 +4265,12 @@ static struct bpf_insn_aux_data *cur_aux + + static int retrieve_ptr_limit(const struct bpf_reg_state *ptr_reg, + const struct bpf_reg_state *off_reg, +- u32 *ptr_limit, u8 opcode) ++ u32 *alu_limit, u8 opcode) + { + bool off_is_neg = off_reg->smin_value < 0; + bool mask_to_left = (opcode == BPF_ADD && off_is_neg) || + (opcode == BPF_SUB && !off_is_neg); +- u32 off, max; ++ u32 off, max = 0, ptr_limit = 0; + + if (!tnum_is_const(off_reg->var_off) && + (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) +@@ -4287,22 +4287,27 @@ static int retrieve_ptr_limit(const stru + */ + off = ptr_reg->off + ptr_reg->var_off.value; + if (mask_to_left) +- *ptr_limit = MAX_BPF_STACK + off; ++ ptr_limit = MAX_BPF_STACK + off; + else +- *ptr_limit = -off - 1; +- return *ptr_limit >= max ? -ERANGE : 0; ++ ptr_limit = -off - 1; ++ break; + case PTR_TO_MAP_VALUE: + max = ptr_reg->map_ptr->value_size; + if (mask_to_left) { +- *ptr_limit = ptr_reg->umax_value + ptr_reg->off; ++ ptr_limit = ptr_reg->umax_value + ptr_reg->off; + } else { + off = ptr_reg->smin_value + ptr_reg->off; +- *ptr_limit = ptr_reg->map_ptr->value_size - off - 1; ++ ptr_limit = ptr_reg->map_ptr->value_size - off - 1; + } +- return *ptr_limit >= max ? -ERANGE : 0; ++ break; + default: + return -EINVAL; + } ++ ++ if (ptr_limit >= max) ++ return -ERANGE; ++ *alu_limit = ptr_limit; ++ return 0; + } + + static bool can_skip_alu_sanitation(const struct bpf_verifier_env *env, diff --git a/queue-5.4/bpf-tighten-speculative-pointer-arithmetic-mask.patch b/queue-5.4/bpf-tighten-speculative-pointer-arithmetic-mask.patch new file mode 100644 index 00000000000..3dbc0b48c12 --- /dev/null +++ b/queue-5.4/bpf-tighten-speculative-pointer-arithmetic-mask.patch @@ -0,0 +1,201 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:38 +0000 +Subject: bpf: Tighten speculative pointer arithmetic mask +To: +Cc: +Message-ID: <20210429220839.15667-8-fllinden@amazon.com> + +From: Daniel Borkmann + +commit 7fedb63a8307dda0ec3b8969a3b233a1dd7ea8e0 upstream. + +This work tightens the offset mask we use for unprivileged pointer arithmetic +in order to mitigate a corner case reported by Piotr and Benedict where in +the speculative domain it is possible to advance, for example, the map value +pointer by up to value_size-1 out-of-bounds in order to leak kernel memory +via side-channel to user space. + +Before this change, the computed ptr_limit for retrieve_ptr_limit() helper +represents largest valid distance when moving pointer to the right or left +which is then fed as aux->alu_limit to generate masking instructions against +the offset register. After the change, the derived aux->alu_limit represents +the largest potential value of the offset register which we mask against which +is just a narrower subset of the former limit. + +For minimal complexity, we call sanitize_ptr_alu() from 2 observation points +in adjust_ptr_min_max_vals(), that is, before and after the simulated alu +operation. In the first step, we retieve the alu_state and alu_limit before +the operation as well as we branch-off a verifier path and push it to the +verification stack as we did before which checks the dst_reg under truncation, +in other words, when the speculative domain would attempt to move the pointer +out-of-bounds. + +In the second step, we retrieve the new alu_limit and calculate the absolute +distance between both. Moreover, we commit the alu_state and final alu_limit +via update_alu_sanitation_state() to the env's instruction aux data, and bail +out from there if there is a mismatch due to coming from different verification +paths with different states. + +Reported-by: Piotr Krysiuk +Reported-by: Benedict Schlueter +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +Tested-by: Benedict Schlueter +[fllinden@amazon.com: backported to 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 73 ++++++++++++++++++++++++++++++-------------------- + 1 file changed, 44 insertions(+), 29 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -4278,7 +4278,7 @@ static int retrieve_ptr_limit(const stru + bool off_is_neg = off_reg->smin_value < 0; + bool mask_to_left = (opcode == BPF_ADD && off_is_neg) || + (opcode == BPF_SUB && !off_is_neg); +- u32 off, max = 0, ptr_limit = 0; ++ u32 max = 0, ptr_limit = 0; + + if (!tnum_is_const(off_reg->var_off) && + (off_reg->smin_value < 0) != (off_reg->smax_value < 0)) +@@ -4287,26 +4287,18 @@ static int retrieve_ptr_limit(const stru + switch (ptr_reg->type) { + case PTR_TO_STACK: + /* Offset 0 is out-of-bounds, but acceptable start for the +- * left direction, see BPF_REG_FP. ++ * left direction, see BPF_REG_FP. Also, unknown scalar ++ * offset where we would need to deal with min/max bounds is ++ * currently prohibited for unprivileged. + */ + max = MAX_BPF_STACK + mask_to_left; +- /* Indirect variable offset stack access is prohibited in +- * unprivileged mode so it's not handled here. +- */ +- off = ptr_reg->off + ptr_reg->var_off.value; +- if (mask_to_left) +- ptr_limit = MAX_BPF_STACK + off; +- else +- ptr_limit = -off - 1; ++ ptr_limit = -(ptr_reg->var_off.value + ptr_reg->off); + break; + case PTR_TO_MAP_VALUE: + max = ptr_reg->map_ptr->value_size; +- if (mask_to_left) { +- ptr_limit = ptr_reg->umax_value + ptr_reg->off; +- } else { +- off = ptr_reg->smin_value + ptr_reg->off; +- ptr_limit = ptr_reg->map_ptr->value_size - off - 1; +- } ++ ptr_limit = (mask_to_left ? ++ ptr_reg->smin_value : ++ ptr_reg->umax_value) + ptr_reg->off; + break; + default: + return REASON_TYPE; +@@ -4361,10 +4353,12 @@ static int sanitize_ptr_alu(struct bpf_v + struct bpf_insn *insn, + const struct bpf_reg_state *ptr_reg, + const struct bpf_reg_state *off_reg, +- struct bpf_reg_state *dst_reg) ++ struct bpf_reg_state *dst_reg, ++ struct bpf_insn_aux_data *tmp_aux, ++ const bool commit_window) + { ++ struct bpf_insn_aux_data *aux = commit_window ? cur_aux(env) : tmp_aux; + struct bpf_verifier_state *vstate = env->cur_state; +- struct bpf_insn_aux_data *aux = cur_aux(env); + bool off_is_neg = off_reg->smin_value < 0; + bool ptr_is_dst_reg = ptr_reg == dst_reg; + u8 opcode = BPF_OP(insn->code); +@@ -4383,18 +4377,33 @@ static int sanitize_ptr_alu(struct bpf_v + if (vstate->speculative) + goto do_sim; + +- alu_state = off_is_neg ? BPF_ALU_NEG_VALUE : 0; +- alu_state |= ptr_is_dst_reg ? +- BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST; +- + err = retrieve_ptr_limit(ptr_reg, off_reg, &alu_limit, opcode); + if (err < 0) + return err; + ++ if (commit_window) { ++ /* In commit phase we narrow the masking window based on ++ * the observed pointer move after the simulated operation. ++ */ ++ alu_state = tmp_aux->alu_state; ++ alu_limit = abs(tmp_aux->alu_limit - alu_limit); ++ } else { ++ alu_state = off_is_neg ? BPF_ALU_NEG_VALUE : 0; ++ alu_state |= ptr_is_dst_reg ? ++ BPF_ALU_SANITIZE_SRC : BPF_ALU_SANITIZE_DST; ++ } ++ + err = update_alu_sanitation_state(aux, alu_state, alu_limit); + if (err < 0) + return err; + do_sim: ++ /* If we're in commit phase, we're done here given we already ++ * pushed the truncated dst_reg into the speculative verification ++ * stack. ++ */ ++ if (commit_window) ++ return 0; ++ + /* Simulate and find potential out-of-bounds access under + * speculative execution from truncation as a result of + * masking when off was not within expected range. If off +@@ -4506,6 +4515,7 @@ static int adjust_ptr_min_max_vals(struc + smin_ptr = ptr_reg->smin_value, smax_ptr = ptr_reg->smax_value; + u64 umin_val = off_reg->umin_value, umax_val = off_reg->umax_value, + umin_ptr = ptr_reg->umin_value, umax_ptr = ptr_reg->umax_value; ++ struct bpf_insn_aux_data tmp_aux = {}; + u8 opcode = BPF_OP(insn->code); + u32 dst = insn->dst_reg; + int ret; +@@ -4564,12 +4574,15 @@ static int adjust_ptr_min_max_vals(struc + !check_reg_sane_offset(env, ptr_reg, ptr_reg->type)) + return -EINVAL; + +- switch (opcode) { +- case BPF_ADD: +- ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); ++ if (sanitize_needed(opcode)) { ++ ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg, ++ &tmp_aux, false); + if (ret < 0) + return sanitize_err(env, insn, ret, off_reg, dst_reg); ++ } + ++ switch (opcode) { ++ case BPF_ADD: + /* We can take a fixed offset as long as it doesn't overflow + * the s32 'off' field + */ +@@ -4620,10 +4633,6 @@ static int adjust_ptr_min_max_vals(struc + } + break; + case BPF_SUB: +- ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg); +- if (ret < 0) +- return sanitize_err(env, insn, ret, off_reg, dst_reg); +- + if (dst_reg == off_reg) { + /* scalar -= pointer. Creates an unknown scalar */ + verbose(env, "R%d tried to subtract pointer from scalar\n", +@@ -4706,6 +4715,12 @@ static int adjust_ptr_min_max_vals(struc + + if (sanitize_check_bounds(env, insn, dst_reg) < 0) + return -EACCES; ++ if (sanitize_needed(opcode)) { ++ ret = sanitize_ptr_alu(env, insn, dst_reg, off_reg, dst_reg, ++ &tmp_aux, true); ++ if (ret < 0) ++ return sanitize_err(env, insn, ret, off_reg, dst_reg); ++ } + + return 0; + } diff --git a/queue-5.4/bpf-update-selftests-to-reflect-new-error-states.patch b/queue-5.4/bpf-update-selftests-to-reflect-new-error-states.patch new file mode 100644 index 00000000000..66be1a18f01 --- /dev/null +++ b/queue-5.4/bpf-update-selftests-to-reflect-new-error-states.patch @@ -0,0 +1,274 @@ +From foo@baz Fri Apr 30 03:37:02 PM CEST 2021 +From: Frank van der Linden +Date: Thu, 29 Apr 2021 22:08:39 +0000 +Subject: bpf: Update selftests to reflect new error states +To: +Cc: +Message-ID: <20210429220839.15667-9-fllinden@amazon.com> + +From: Daniel Borkmann + +commit d7a5091351756d0ae8e63134313c455624e36a13 upstream. + +Update various selftest error messages: + + * The 'Rx tried to sub from different maps, paths, or prohibited types' + is reworked into more specific/differentiated error messages for better + guidance. + + * The change into 'value -4294967168 makes map_value pointer be out of + bounds' is due to moving the mixed bounds check into the speculation + handling and thus occuring slightly later than above mentioned sanity + check. + + * The change into 'math between map_value pointer and register with + unbounded min value' is similarly due to register sanity check coming + before the mixed bounds check. + + * The case of 'map access: known scalar += value_ptr from different maps' + now loads fine given masks are the same from the different paths (despite + max map value size being different). + +Signed-off-by: Daniel Borkmann +Reviewed-by: John Fastabend +Acked-by: Alexei Starovoitov +[fllinden@amazon - skip bounds.c test mods, they won't change error msg on 5.4] +Signed-off-by: Frank van der Linden +Signed-off-by: Greg Kroah-Hartman +--- + tools/testing/selftests/bpf/verifier/bounds_deduction.c | 21 +++++----- + tools/testing/selftests/bpf/verifier/bounds_mix_sign_unsign.c | 13 ------ + tools/testing/selftests/bpf/verifier/unpriv.c | 2 + tools/testing/selftests/bpf/verifier/value_ptr_arith.c | 6 -- + 4 files changed, 14 insertions(+), 28 deletions(-) + +--- a/tools/testing/selftests/bpf/verifier/bounds_deduction.c ++++ b/tools/testing/selftests/bpf/verifier/bounds_deduction.c +@@ -6,7 +6,7 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R0 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "R0 tried to subtract pointer from scalar", + .result = REJECT, + }, +@@ -21,7 +21,7 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_1, BPF_REG_0), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R1 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .result_unpriv = REJECT, + .result = ACCEPT, + .retval = 1, +@@ -34,22 +34,23 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R0 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "R0 tried to subtract pointer from scalar", + .result = REJECT, + }, + { + "check deducing bounds from const, 4", + .insns = { ++ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1), + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_JMP_IMM(BPF_JSLE, BPF_REG_0, 0, 1), + BPF_EXIT_INSN(), + BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 0, 1), + BPF_EXIT_INSN(), +- BPF_ALU64_REG(BPF_SUB, BPF_REG_1, BPF_REG_0), ++ BPF_ALU64_REG(BPF_SUB, BPF_REG_6, BPF_REG_0), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R1 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R6 has pointer with unsupported alu operation", + .result_unpriv = REJECT, + .result = ACCEPT, + }, +@@ -61,7 +62,7 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R0 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "R0 tried to subtract pointer from scalar", + .result = REJECT, + }, +@@ -74,7 +75,7 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R0 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "R0 tried to subtract pointer from scalar", + .result = REJECT, + }, +@@ -88,7 +89,7 @@ + offsetof(struct __sk_buff, mark)), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R1 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "dereference of modified ctx ptr", + .result = REJECT, + .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS, +@@ -103,7 +104,7 @@ + offsetof(struct __sk_buff, mark)), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R1 tried to add from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "dereference of modified ctx ptr", + .result = REJECT, + .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS, +@@ -116,7 +117,7 @@ + BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R0 tried to sub from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 has pointer with unsupported alu operation", + .errstr = "R0 tried to subtract pointer from scalar", + .result = REJECT, + }, +--- a/tools/testing/selftests/bpf/verifier/bounds_mix_sign_unsign.c ++++ b/tools/testing/selftests/bpf/verifier/bounds_mix_sign_unsign.c +@@ -19,7 +19,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -43,7 +42,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -69,7 +67,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R8 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -94,7 +91,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R8 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -141,7 +137,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -210,7 +205,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -260,7 +254,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -287,7 +280,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -313,7 +305,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -342,7 +333,6 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R7 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -372,7 +362,6 @@ + }, + .fixup_map_hash_8b = { 4 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, + }, + { +@@ -400,7 +389,5 @@ + }, + .fixup_map_hash_8b = { 3 }, + .errstr = "unbounded min value", +- .errstr_unpriv = "R1 has unknown scalar with mixed signed bounds", + .result = REJECT, +- .result_unpriv = REJECT, + }, +--- a/tools/testing/selftests/bpf/verifier/unpriv.c ++++ b/tools/testing/selftests/bpf/verifier/unpriv.c +@@ -503,7 +503,7 @@ + BPF_STX_MEM(BPF_DW, BPF_REG_1, BPF_REG_0, -8), + BPF_EXIT_INSN(), + }, +- .errstr_unpriv = "R1 tried to add from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R1 stack pointer arithmetic goes out of range", + .result_unpriv = REJECT, + .result = ACCEPT, + }, +--- a/tools/testing/selftests/bpf/verifier/value_ptr_arith.c ++++ b/tools/testing/selftests/bpf/verifier/value_ptr_arith.c +@@ -21,8 +21,6 @@ + .fixup_map_hash_16b = { 5 }, + .fixup_map_array_48b = { 8 }, + .result = ACCEPT, +- .result_unpriv = REJECT, +- .errstr_unpriv = "R1 tried to add from different maps", + .retval = 1, + }, + { +@@ -122,7 +120,7 @@ + .fixup_map_array_48b = { 1 }, + .result = ACCEPT, + .result_unpriv = REJECT, +- .errstr_unpriv = "R2 tried to add from different pointers or scalars", ++ .errstr_unpriv = "R2 tried to add from different maps, paths or scalars", + .retval = 0, + }, + { +@@ -169,7 +167,7 @@ + .fixup_map_array_48b = { 1 }, + .result = ACCEPT, + .result_unpriv = REJECT, +- .errstr_unpriv = "R2 tried to add from different maps, paths, or prohibited types", ++ .errstr_unpriv = "R2 tried to add from different maps, paths or scalars", + .retval = 0, + }, + { diff --git a/queue-5.4/series b/queue-5.4/series new file mode 100644 index 00000000000..8667ccffb40 --- /dev/null +++ b/queue-5.4/series @@ -0,0 +1,8 @@ +bpf-move-off_reg-into-sanitize_ptr_alu.patch +bpf-ensure-off_reg-has-no-mixed-signed-bounds-for-all-types.patch +bpf-rework-ptr_limit-into-alu_limit-and-add-common-error-path.patch +bpf-improve-verifier-error-messages-for-users.patch +bpf-refactor-and-streamline-bounds-check-into-helper.patch +bpf-move-sanitize_val_alu-out-of-op-switch.patch +bpf-tighten-speculative-pointer-arithmetic-mask.patch +bpf-update-selftests-to-reflect-new-error-states.patch -- 2.47.3