From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B299BC433EF for ; Thu, 30 Dec 2021 02:37:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234991AbhL3Ch3 (ORCPT ); Wed, 29 Dec 2021 21:37:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52656 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234985AbhL3ChX (ORCPT ); Wed, 29 Dec 2021 21:37:23 -0500 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 87CA6C061574; Wed, 29 Dec 2021 18:37:21 -0800 (PST) Received: by mail-pj1-x1042.google.com with SMTP id o63-20020a17090a0a4500b001b1c2db8145so26628423pjo.5; Wed, 29 Dec 2021 18:37:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=jEXwUNhoHZCsBrnj01xVDNDmik0sYD9mhvi5LKMLylk=; b=PBj9nHxLg+4tM2zDhOeSuFotghwW2by1OOEJBGKezyv3ZDbAAkZlle55nth+CzT9MY GWZXLH7sseo79RNqGyU572iU+L5+eHEaCXtNtjV61htR3aNBJ7GMhRTteKvoV2K2FJyk pCwI+QLzFJvUMZnVmKaOsXzTZUQUglJH100jMOEnsHRAb2l+jFvjd24tVZDiyxCNscN+ yOi8vKZ3A6n//VjhJovcqVSQ5PT+ZcXSOXdizOtmJuZjiHtvC1JgpRyXb9JrDwN2eg1K F7FDSvBla9ooFHargVzYeYRv24O07YApDSPIrR5m7rqVtOjtbSZf4thrmGtRGeBqaSU/ dqbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=jEXwUNhoHZCsBrnj01xVDNDmik0sYD9mhvi5LKMLylk=; b=bkklEUhwXSZwjGj6vLfufuTax8Ly+HBhQ/LK/vpMDYWeGRYQcoOMwW9fjcBNsgydEy 2PWFpurVL0ZU9wy/64X619uy/oxkU+sfvM/+f5CS3s1u8mGividrj2Jxi7DGfvo/ns/N oxU/2qPrFhElPfbdZ/c9ngVlwZMe5lCbMnZp0oZiFwq7AwDC98OjylP3QVyy/8b+Td9Q daKPvdOIBYKULCr3qit2U8RgX9JOe8EstehyXs9kgbT0nsV+hgYFUxk0+9aILn0KGI5m JeGaIm7jwhmQr5I3HiJrap8ppgd1JOSwgQg5q6hggVkw+9GeK7VCAHjd5ueNhdPDsB0q 8gEA== X-Gm-Message-State: AOAM533U7T6e9gQIfSXfdAodT9uvhkVbIqfZdPC4viav3llMa5Afl3Bh 6mSQO1YwhQGru+AbH9ajo78YMzjZm1I= X-Google-Smtp-Source: ABdhPJzmLoSpuOQosL1+V6qUrdUaOQiuSVGW4cgwxR7+RXK5n/ZTdyCi+n3obtDlWLYkYRL4AbfkQA== X-Received: by 2002:a17:902:76c6:b0:142:644e:e9a with SMTP id j6-20020a17090276c600b00142644e0e9amr29293481plt.6.1640831840863; Wed, 29 Dec 2021 18:37:20 -0800 (PST) Received: from localhost ([2405:201:6014:d064:3d4e:6265:800c:dc84]) by smtp.gmail.com with ESMTPSA id w2sm21044563pgt.93.2021.12.29.18.37.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Dec 2021 18:37:20 -0800 (PST) From: Kumar Kartikeya Dwivedi To: bpf@vger.kernel.org, netdev@vger.kernel.org, netfilter-devel@vger.kernel.org Cc: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , Maxim Mikityanskiy , Pablo Neira Ayuso , Florian Westphal , Jesper Dangaard Brouer , =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= Subject: [PATCH bpf-next v5 4/9] bpf: Introduce mem, size argument pair support for kfunc Date: Thu, 30 Dec 2021 08:07:00 +0530 Message-Id: <20211230023705.3860970-5-memxor@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20211230023705.3860970-1-memxor@gmail.com> References: <20211230023705.3860970-1-memxor@gmail.com> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=9984; h=from:subject; bh=e/lR+ACSoqgOoK477Yw0DLfFvDSkrxiQFZEosa1LORY=; b=owEBbQKS/ZANAwAIAUzgyIZIvxHKAcsmYgBhzRr9qybc9Aug1rqBJ+Ra56E+2Ogb9eGRI5srnAAq dV4LFLKJAjMEAAEIAB0WIQRLvip+Buz51YI8YRFM4MiGSL8RygUCYc0a/QAKCRBM4MiGSL8RykgDD/ 0cNn1bIrOKkxaPO50FM0pjWoGeVPLvrxAsKPXpLvKxhKTdq1eH0tqkwG89xOpgXXPd/HZVIjq0f1s+ cvxao0lXTjumDCgSvxAEf8MSMxc7/cxGIVoxD2hnhvfnVODYJZFLELssEXvjdY2Adf6QspcE2RtRJf QM1jdaXxdfePXNJtK1wmaigELIi0xQFxmAk9zV/wZLsX3vftN7xI5GL+TcUMLfsziHRHqQO9jNO05c HaqTz37WTBmXErF5WI2CCa+VtD6xLjqiUpeDmHZmFgDDq5mjmlNN5CyFXMxaFm6Zxyyzp5kRmzTqKl BKlojJyV5BkENoRAjwZhQN5k2hr2s4il5rbF/wKdN5IbhWpsNcIQ/GKagfSfqUjAoSMpA4kh7Oc6aS kqqTyEYDnOLNVpUEQoOh/4xn3bfBGaIGeENpFVkT0Z35FmMknEy/g7IxeyT79ZBntL2UnpkRPkuypZ XBde1rdYoI11Ho+Fyg+G0FoyrgUMsbrod9Dqc88GhkecnuUpXpObDZIRU0zMMt2TcTB7ejZ/F6zzg2 OywwO6epBIHkLe1k/uQKeaHM/sIbDhowrSuT0ODS7j+FtLnwe2cZ0dCfYgQEXKmDeHms0ZjcMYAkAE b3g7YJZnYcQfW1fFz3jg+UGdOLpDDqVEMI4/GshqgQexwscNOaGrYobe92Dg== X-Developer-Key: i=memxor@gmail.com; a=openpgp; fpr=4BBE2A7E06ECF9D5823C61114CE0C88648BF11CA Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org BPF helpers can associate two adjacent arguments together to pass memory of certain size, using ARG_PTR_TO_MEM and ARG_CONST_SIZE arguments. Since we don't use bpf_func_proto for kfunc, we need to leverage BTF to implement similar support. The ARG_CONST_SIZE processing for helpers is refactored into a common check_mem_size_reg helper that is shared with kfunc as well. kfunc ptr_to_mem support follows logic similar to global functions, where verification is done as if pointer is not null, even when it may be null. This leads to a simple to follow rule for writing kfunc: always check the argument pointer for NULL, except when it is PTR_TO_CTX. Also, the PTR_TO_CTX case is also only safe when the helper expecting pointer to program ctx is not exposed to other programs where same struct is not ctx type. In that case, the type check will fall through to other cases and would permit passing other types of pointers, possibly NULL at runtime. Currently, we require the size argument to be suffixed with "__sz" in the parameter name. This information is then recorded in kernel BTF and verified during function argument checking. In the future we can use BTF tagging instead, and modify the kernel function definitions. This will be a purely kernel-side change. This allows us to have some form of backwards compatibility for structures that are passed in to the kernel function with their size, and allow variable length structures to be passed in if they are accompanied by a size parameter. Signed-off-by: Kumar Kartikeya Dwivedi --- include/linux/bpf_verifier.h | 2 + kernel/bpf/btf.c | 46 ++++++++++++- kernel/bpf/verifier.c | 124 ++++++++++++++++++++++------------- 3 files changed, 124 insertions(+), 48 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 143401d4c9d9..857fd687bdc2 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -521,6 +521,8 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt); int check_ctx_reg(struct bpf_verifier_env *env, const struct bpf_reg_state *reg, int regno); +int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + u32 regno); int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 regno, u32 mem_size); diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index f5b3049a56e0..fc1142a044c4 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5887,6 +5887,30 @@ static bool __btf_type_is_scalar_struct(struct bpf_verifier_log *log, return true; } +static bool is_kfunc_arg_mem_size(const struct btf *btf, + const struct btf_param *arg, + const struct bpf_reg_state *reg) +{ + int len, sfx_len = sizeof("__sz") - 1; + const struct btf_type *t; + const char *param_name; + + t = btf_type_skip_modifiers(btf, arg->type, NULL); + if (!btf_type_is_scalar(t) || reg->type != SCALAR_VALUE) + return false; + + /* In the future, this can be ported to use BTF tagging */ + param_name = btf_name_by_offset(btf, arg->name_off); + len = strlen(param_name); + if (len < sfx_len) + return false; + param_name += len - sfx_len; + if (strncmp(param_name, "__sz", sfx_len)) + return false; + + return true; +} + static int btf_check_func_arg_match(struct bpf_verifier_env *env, const struct btf *btf, u32 func_id, struct bpf_reg_state *regs, @@ -5998,17 +6022,33 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, u32 type_size; if (is_kfunc) { + bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], ®s[regno + 1]); + /* Permit pointer to mem, but only when argument * type is pointer to scalar, or struct composed * (recursively) of scalars. + * When arg_mem_size is true, the pointer can be + * void *. */ if (!btf_type_is_scalar(ref_t) && - !__btf_type_is_scalar_struct(log, btf, ref_t, 0)) { + !__btf_type_is_scalar_struct(log, btf, ref_t, 0) && + (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) { bpf_log(log, - "arg#%d pointer type %s %s must point to scalar or struct with scalar\n", - i, btf_type_str(ref_t), ref_tname); + "arg#%d pointer type %s %s must point to %sscalar, or struct with scalar\n", + i, btf_type_str(ref_t), ref_tname, arg_mem_size ? "void, " : ""); return -EINVAL; } + + /* Check for mem, len pair */ + if (arg_mem_size) { + if (check_kfunc_mem_size_reg(env, ®s[regno + 1], regno + 1)) { + bpf_log(log, "arg#%d arg#%d memory, len pair leads to invalid memory access\n", + i, i + 1); + return -EINVAL; + } + i++; + continue; + } } resolve_ret = btf_resolve_size(btf, ref_t, &type_size); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e9503192101f..7ec13d146b05 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4849,6 +4849,62 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno, } } +static int check_mem_size_reg(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, u32 regno, + bool zero_size_allowed, + struct bpf_call_arg_meta *meta) +{ + int err; + + /* This is used to refine r0 return value bounds for helpers + * that enforce this value as an upper bound on return values. + * See do_refine_retval_range() for helpers that can refine + * the return value. C type of helper is u32 so we pull register + * bound from umax_value however, if negative verifier errors + * out. Only upper bounds can be learned because retval is an + * int type and negative retvals are allowed. + */ + if (meta) + meta->msize_max_value = reg->umax_value; + + /* The register is SCALAR_VALUE; the access check + * happens using its boundaries. + */ + if (!tnum_is_const(reg->var_off)) + /* For unprivileged variable accesses, disable raw + * mode so that the program is required to + * initialize all the memory that the helper could + * just partially fill up. + */ + meta = NULL; + + if (reg->smin_value < 0) { + verbose(env, "R%d min value is negative, either use unsigned or 'var &= const'\n", + regno); + return -EACCES; + } + + if (reg->umin_value == 0) { + err = check_helper_mem_access(env, regno - 1, 0, + zero_size_allowed, + meta); + if (err) + return err; + } + + if (reg->umax_value >= BPF_MAX_VAR_SIZ) { + verbose(env, "R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n", + regno); + return -EACCES; + } + err = check_helper_mem_access(env, regno - 1, + reg->umax_value, + zero_size_allowed, meta); + if (!err) + err = mark_chain_precision(env, regno); + return err; +} + int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, u32 regno, u32 mem_size) { @@ -4872,6 +4928,28 @@ int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, return check_helper_mem_access(env, regno, mem_size, true, NULL); } +int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, + u32 regno) +{ + struct bpf_reg_state *mem_reg = &cur_regs(env)[regno - 1]; + bool may_be_null = type_may_be_null(mem_reg->type); + struct bpf_reg_state saved_reg; + int err; + + WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5); + + if (may_be_null) { + saved_reg = *mem_reg; + mark_ptr_not_null_reg(mem_reg); + } + + err = check_mem_size_reg(env, reg, regno, true, NULL); + + if (may_be_null) + *mem_reg = saved_reg; + return err; +} + /* Implementation details: * bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL * Two bpf_map_lookups (even with the same key) will have different reg->id. @@ -5393,51 +5471,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, } else if (arg_type_is_mem_size(arg_type)) { bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO); - /* This is used to refine r0 return value bounds for helpers - * that enforce this value as an upper bound on return values. - * See do_refine_retval_range() for helpers that can refine - * the return value. C type of helper is u32 so we pull register - * bound from umax_value however, if negative verifier errors - * out. Only upper bounds can be learned because retval is an - * int type and negative retvals are allowed. - */ - meta->msize_max_value = reg->umax_value; - - /* The register is SCALAR_VALUE; the access check - * happens using its boundaries. - */ - if (!tnum_is_const(reg->var_off)) - /* For unprivileged variable accesses, disable raw - * mode so that the program is required to - * initialize all the memory that the helper could - * just partially fill up. - */ - meta = NULL; - - if (reg->smin_value < 0) { - verbose(env, "R%d min value is negative, either use unsigned or 'var &= const'\n", - regno); - return -EACCES; - } - - if (reg->umin_value == 0) { - err = check_helper_mem_access(env, regno - 1, 0, - zero_size_allowed, - meta); - if (err) - return err; - } - - if (reg->umax_value >= BPF_MAX_VAR_SIZ) { - verbose(env, "R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n", - regno); - return -EACCES; - } - err = check_helper_mem_access(env, regno - 1, - reg->umax_value, - zero_size_allowed, meta); - if (!err) - err = mark_chain_precision(env, regno); + err = check_mem_size_reg(env, reg, regno, zero_size_allowed, meta); } else if (arg_type_is_alloc_size(arg_type)) { if (!tnum_is_const(reg->var_off)) { verbose(env, "R%d is not a known constant'\n", -- 2.34.1