On Mon, Nov 14, 2022 at 4:01 PM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> After previous commit, we are minimizing helper specific assumptions
> from check_func_arg_reg_off, making it generic, and offloading checks
> for a specific argument type to their respective functions called after
> check_func_arg_reg_off has been called.
>
> This allows relying on a consistent set of guarantees after that call
> and then relying on them in code that deals with registers for each
> argument type later. This is in line with how process_spin_lock,
> process_timer_func, process_kptr_func check reg->var_off to be constant.
> The same reasoning is used here to move the alignment check into
> process_dynptr_func. Note that it also needs to check for constant
> var_off, and accumulate the constant var_off when computing the spi in
> get_spi, but that fix will come in later changes.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
Acked-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> kernel/bpf/verifier.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 34e67d04579b..fd292f762d53 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5774,6 +5774,14 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno,
> return -EFAULT;
> }
>
> + /* CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to
> + * check_func_arg_reg_off's logic. We only need to check offset
> + * alignment for PTR_TO_STACK.
> + */
> + if (reg->type == PTR_TO_STACK && (reg->off % BPF_REG_SIZE)) {
> + verbose(env, "cannot pass in dynptr at an offset=%d\n", reg->off);
> + return -EINVAL;
> + }
> /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to a
> * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*):
> *
> @@ -6125,11 +6133,6 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
> switch (type) {
> /* Pointer types where both fixed and variable offset is explicitly allowed: */
> case PTR_TO_STACK:
> - if (arg_type_is_dynptr(arg_type) && reg->off % BPF_REG_SIZE) {
> - verbose(env, "cannot pass in dynptr at an offset\n");
> - return -EINVAL;
> - }
> - fallthrough;
> case PTR_TO_PACKET:
> case PTR_TO_PACKET_META:
> case PTR_TO_MAP_KEY:
> --
> 2.38.1
>
