Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH v9 03/10] btf: Handle dynamic pointer parameter in kfuncs
To:     Roberto Sassu <roberto.sassu@huawei.com>, ast@kernel.org,
        andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
        yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
        sdf@google.com, haoluo@google.com, jolsa@kernel.org,
        corbet@lwn.net, dhowells@redhat.com, jarkko@kernel.org,
        rostedt@goodmis.org, mingo@redhat.com, paul@paul-moore.com,
        jmorris@namei.org, serge@hallyn.com, shuah@kernel.org
Cc:     bpf@vger.kernel.org, linux-doc@vger.kernel.org,
        keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        Joanne Koong <joannelkoong@gmail.com>
References: <20220809134603.1769279-1-roberto.sassu@huawei.com>
 <20220809134603.1769279-4-roberto.sassu@huawei.com>
From:   Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <6fca828f-9117-2a1a-b386-6167fc3384aa@iogearbox.net>
Date:   Wed, 10 Aug 2022 00:29:28 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
MIME-Version: 1.0
In-Reply-To: <20220809134603.1769279-4-roberto.sassu@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/9/22 3:45 PM, Roberto Sassu wrote:
[...]
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 67dfc728fbf8..17cca396c89f 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6363,6 +6363,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>   
>   			if (is_kfunc) {
>   				bool arg_mem_size = i + 1 < nargs && is_kfunc_arg_mem_size(btf, &args[i + 1], &regs[regno + 1]);
> +				bool arg_dynptr = btf_type_is_struct(ref_t) &&
> +						  !strcmp(ref_tname, "bpf_dynptr_kern");
>   
>   				/* Permit pointer to mem, but only when argument
>   				 * type is pointer to scalar, or struct composed
> @@ -6372,6 +6374,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>   				 */
>   				if (!btf_type_is_scalar(ref_t) &&
>   				    !__btf_type_is_scalar_struct(log, btf, ref_t, 0) &&
> +				    !arg_dynptr &&
>   				    (arg_mem_size ? !btf_type_is_void(ref_t) : 1)) {
>   					bpf_log(log,
>   						"arg#%d pointer type %s %s must point to %sscalar, or struct with scalar\n",
> @@ -6379,6 +6382,20 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
>   					return -EINVAL;
>   				}
>   
> +				/* Assume initialized dynptr. */

This comment is a bit misleading, too, given we don't assume but enforce it. I'd probably
just fold this into above one where we permit pointer to mem given the test there gets
extended anyway, so the comment should be in line with the tests.

> +				if (arg_dynptr) {
> +					if (!is_dynptr_reg_valid_init(env, reg,
> +							ARG_PTR_TO_DYNPTR)) {
> +						bpf_log(log,
> +							"arg#%d pointer type %s %s must be initialized\n",
> +							i, btf_type_str(ref_t),
> +							ref_tname);
> +						return -EINVAL;
> +					}
> +
> +					continue;
> +				}
> +
>   				/* Check for mem, len pair */
>   				if (arg_mem_size) {
>   					if (check_kfunc_mem_size_reg(env, &regs[regno + 1], regno + 1)) {