|
| 1 | +From aa934184cf90b7fe3a8ce0b481d6a512e0f96e9b Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kumar Kartikeya Dwivedi <memxor@gmail.com> |
| 3 | +Date: Sat, 25 Jun 2022 09:15:40 +0300 |
| 4 | +Subject: [PATCH] bpf: Introduce mem, size argument pair support for kfunc |
| 5 | + |
| 6 | +BPF helpers can associate two adjacent arguments together to pass memory |
| 7 | +of certain size, using ARG_PTR_TO_MEM and ARG_CONST_SIZE arguments. |
| 8 | +Since we don't use bpf_func_proto for kfunc, we need to leverage BTF to |
| 9 | +implement similar support. |
| 10 | + |
| 11 | +The ARG_CONST_SIZE processing for helpers is refactored into a common |
| 12 | +check_mem_size_reg helper that is shared with kfunc as well. kfunc |
| 13 | +ptr_to_mem support follows logic similar to global functions, where |
| 14 | +verification is done as if pointer is not null, even when it may be |
| 15 | +null. |
| 16 | + |
| 17 | +This leads to a simple to follow rule for writing kfunc: always check |
| 18 | +the argument pointer for NULL, except when it is PTR_TO_CTX. Also, the |
| 19 | +PTR_TO_CTX case is also only safe when the helper expecting pointer to |
| 20 | +program ctx is not exposed to other programs where same struct is not |
| 21 | +ctx type. In that case, the type check will fall through to other cases |
| 22 | +and would permit passing other types of pointers, possibly NULL at |
| 23 | +runtime. |
| 24 | + |
| 25 | +Currently, we require the size argument to be suffixed with "__sz" in |
| 26 | +the parameter name. This information is then recorded in kernel BTF and |
| 27 | +verified during function argument checking. In the future we can use BTF |
| 28 | +tagging instead, and modify the kernel function definitions. This will |
| 29 | +be a purely kernel-side change. |
| 30 | + |
| 31 | +This allows us to have some form of backwards compatibility for |
| 32 | +structures that are passed in to the kernel function with their size, |
| 33 | +and allow variable length structures to be passed in if they are |
| 34 | +accompanied by a size parameter. |
| 35 | + |
| 36 | +Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> |
| 37 | +Link: https://lore.kernel.org/r/20220114163953.1455836-5-memxor@gmail.com |
| 38 | +Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| 39 | +(cherry picked from d583691c47dc0424ebe926000339a6d6cd590ff7) |
| 40 | +Signed-off-by: Sergey Nizovtsev <sn@tempesta-tech.com> |
| 41 | +--- |
| 42 | + include/linux/bpf_verifier.h | 2 + |
| 43 | + kernel/bpf/btf.c | 48 +++++++++++++++++++-- |
| 44 | + kernel/bpf/verifier.c | 83 ++++++++++++++++++++++++++++++++++++ |
| 45 | + 3 files changed, 130 insertions(+), 3 deletions(-) |
| 46 | + |
| 47 | +diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h |
| 48 | +index e9993172f892e..8cc8633647246 100644 |
| 49 | +--- a/include/linux/bpf_verifier.h |
| 50 | ++++ b/include/linux/bpf_verifier.h |
| 51 | +@@ -521,6 +521,8 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt); |
| 52 | + |
| 53 | + int check_ptr_off_reg(struct bpf_verifier_env *env, |
| 54 | + const struct bpf_reg_state *reg, int regno); |
| 55 | ++int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, |
| 56 | ++ u32 regno); |
| 57 | + int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, |
| 58 | + u32 regno, u32 mem_size); |
| 59 | + |
| 60 | +diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c |
| 61 | +index ac89e65d1692e..30e72cc2744ed 100644 |
| 62 | +--- a/kernel/bpf/btf.c |
| 63 | ++++ b/kernel/bpf/btf.c |
| 64 | +@@ -5629,6 +5629,32 @@ static bool __btf_type_is_scalar_struct(struct bpf_verifier_log *log, |
| 65 | + return true; |
| 66 | + } |
| 67 | + |
| 68 | ++static bool is_kfunc_arg_mem_size(const struct btf *btf, |
| 69 | ++ const struct btf_param *arg, |
| 70 | ++ const struct bpf_reg_state *reg) |
| 71 | ++{ |
| 72 | ++ int len, sfx_len = sizeof("__sz") - 1; |
| 73 | ++ const struct btf_type *t; |
| 74 | ++ const char *param_name; |
| 75 | ++ |
| 76 | ++ t = btf_type_skip_modifiers(btf, arg->type, NULL); |
| 77 | ++ if (!btf_type_is_scalar(t) || reg->type != SCALAR_VALUE) |
| 78 | ++ return false; |
| 79 | ++ |
| 80 | ++ /* In the future, this can be ported to use BTF tagging */ |
| 81 | ++ param_name = btf_name_by_offset(btf, arg->name_off); |
| 82 | ++ if (str_is_empty(param_name)) |
| 83 | ++ return false; |
| 84 | ++ len = strlen(param_name); |
| 85 | ++ if (len < sfx_len) |
| 86 | ++ return false; |
| 87 | ++ param_name += len - sfx_len; |
| 88 | ++ if (strncmp(param_name, "__sz", sfx_len)) |
| 89 | ++ return false; |
| 90 | ++ |
| 91 | ++ return true; |
| 92 | ++} |
| 93 | ++ |
| 94 | + static int btf_check_func_arg_match(struct bpf_verifier_env *env, |
| 95 | + const struct btf *btf, u32 func_id, |
| 96 | + struct bpf_reg_state *regs, |
| 97 | +@@ -5741,17 +5767,33 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, |
| 98 | + u32 type_size; |
| 99 | + |
| 100 | + if (is_kfunc) { |
| 101 | ++ bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], ®s[regno + 1]); |
| 102 | ++ |
| 103 | + /* Permit pointer to mem, but only when argument |
| 104 | + * type is pointer to scalar, or struct composed |
| 105 | + * (recursively) of scalars. |
| 106 | ++ * When arg_mem_size is true, the pointer can be |
| 107 | ++ * void *. |
| 108 | + */ |
| 109 | + if (!btf_type_is_scalar(ref_t) && |
| 110 | +- !__btf_type_is_scalar_struct(log, btf, ref_t, 0)) { |
| 111 | ++ !__btf_type_is_scalar_struct(log, btf, ref_t, 0) && |
| 112 | ++ (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) { |
| 113 | + bpf_log(log, |
| 114 | +- "arg#%d pointer type %s %s must point to scalar or struct with scalar\n", |
| 115 | +- i, btf_type_str(ref_t), ref_tname); |
| 116 | ++ "arg#%d pointer type %s %s must point to %sscalar, or struct with scalar\n", |
| 117 | ++ i, btf_type_str(ref_t), ref_tname, arg_mem_size ? "void, " : ""); |
| 118 | + return -EINVAL; |
| 119 | + } |
| 120 | ++ |
| 121 | ++ /* Check for mem, len pair */ |
| 122 | ++ if (arg_mem_size) { |
| 123 | ++ if (check_kfunc_mem_size_reg(env, ®s[regno + 1], regno + 1)) { |
| 124 | ++ bpf_log(log, "arg#%d arg#%d memory, len pair leads to invalid memory access\n", |
| 125 | ++ i, i + 1); |
| 126 | ++ return -EINVAL; |
| 127 | ++ } |
| 128 | ++ i++; |
| 129 | ++ continue; |
| 130 | ++ } |
| 131 | + } |
| 132 | + |
| 133 | + resolve_ret = btf_resolve_size(btf, ref_t, &type_size); |
| 134 | +diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c |
| 135 | +index 984c5e446e570..55826810bf581 100644 |
| 136 | +--- a/kernel/bpf/verifier.c |
| 137 | ++++ b/kernel/bpf/verifier.c |
| 138 | +@@ -4892,6 +4892,61 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno, |
| 139 | + } |
| 140 | + } |
| 141 | + |
| 142 | ++static int check_mem_size_reg(struct bpf_verifier_env *env, |
| 143 | ++ struct bpf_reg_state *reg, u32 regno, |
| 144 | ++ bool zero_size_allowed, |
| 145 | ++ struct bpf_call_arg_meta *meta) |
| 146 | ++{ |
| 147 | ++ int err; |
| 148 | ++ |
| 149 | ++ /* This is used to refine r0 return value bounds for helpers |
| 150 | ++ * that enforce this value as an upper bound on return values. |
| 151 | ++ * See do_refine_retval_range() for helpers that can refine |
| 152 | ++ * the return value. C type of helper is u32 so we pull register |
| 153 | ++ * bound from umax_value however, if negative verifier errors |
| 154 | ++ * out. Only upper bounds can be learned because retval is an |
| 155 | ++ * int type and negative retvals are allowed. |
| 156 | ++ */ |
| 157 | ++ meta->msize_max_value = reg->umax_value; |
| 158 | ++ |
| 159 | ++ /* The register is SCALAR_VALUE; the access check |
| 160 | ++ * happens using its boundaries. |
| 161 | ++ */ |
| 162 | ++ if (!tnum_is_const(reg->var_off)) |
| 163 | ++ /* For unprivileged variable accesses, disable raw |
| 164 | ++ * mode so that the program is required to |
| 165 | ++ * initialize all the memory that the helper could |
| 166 | ++ * just partially fill up. |
| 167 | ++ */ |
| 168 | ++ meta = NULL; |
| 169 | ++ |
| 170 | ++ if (reg->smin_value < 0) { |
| 171 | ++ verbose(env, "R%d min value is negative, either use unsigned or 'var &= const'\n", |
| 172 | ++ regno); |
| 173 | ++ return -EACCES; |
| 174 | ++ } |
| 175 | ++ |
| 176 | ++ if (reg->umin_value == 0) { |
| 177 | ++ err = check_helper_mem_access(env, regno - 1, 0, |
| 178 | ++ zero_size_allowed, |
| 179 | ++ meta); |
| 180 | ++ if (err) |
| 181 | ++ return err; |
| 182 | ++ } |
| 183 | ++ |
| 184 | ++ if (reg->umax_value >= BPF_MAX_VAR_SIZ) { |
| 185 | ++ verbose(env, "R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n", |
| 186 | ++ regno); |
| 187 | ++ return -EACCES; |
| 188 | ++ } |
| 189 | ++ err = check_helper_mem_access(env, regno - 1, |
| 190 | ++ reg->umax_value, |
| 191 | ++ zero_size_allowed, meta); |
| 192 | ++ if (!err) |
| 193 | ++ err = mark_chain_precision(env, regno); |
| 194 | ++ return err; |
| 195 | ++} |
| 196 | ++ |
| 197 | + int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, |
| 198 | + u32 regno, u32 mem_size) |
| 199 | + { |
| 200 | +@@ -4915,6 +4970,34 @@ int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, |
| 201 | + return check_helper_mem_access(env, regno, mem_size, true, NULL); |
| 202 | + } |
| 203 | + |
| 204 | ++int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, |
| 205 | ++ u32 regno) |
| 206 | ++{ |
| 207 | ++ struct bpf_reg_state *mem_reg = &cur_regs(env)[regno - 1]; |
| 208 | ++ bool may_be_null = type_may_be_null(mem_reg->type); |
| 209 | ++ struct bpf_reg_state saved_reg; |
| 210 | ++ struct bpf_call_arg_meta meta; |
| 211 | ++ int err; |
| 212 | ++ |
| 213 | ++ WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5); |
| 214 | ++ |
| 215 | ++ memset(&meta, 0, sizeof(meta)); |
| 216 | ++ |
| 217 | ++ if (may_be_null) { |
| 218 | ++ saved_reg = *mem_reg; |
| 219 | ++ mark_ptr_not_null_reg(mem_reg); |
| 220 | ++ } |
| 221 | ++ |
| 222 | ++ err = check_mem_size_reg(env, reg, regno, true, &meta); |
| 223 | ++ /* Check access for BPF_WRITE */ |
| 224 | ++ meta.raw_mode = true; |
| 225 | ++ err = err ?: check_mem_size_reg(env, reg, regno, true, &meta); |
| 226 | ++ |
| 227 | ++ if (may_be_null) |
| 228 | ++ *mem_reg = saved_reg; |
| 229 | ++ return err; |
| 230 | ++} |
| 231 | ++ |
| 232 | + /* Implementation details: |
| 233 | + * bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL |
| 234 | + * Two bpf_map_lookups (even with the same key) will have different reg->id. |
| 235 | +-- |
| 236 | +2.38.0 |
| 237 | + |
0 commit comments