linux-kernel - Re: [PATCH bpf-next v9 2/4] bpf: Allow trusted pointers to be passed to KF_TRUSTED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221120172815.godn2rt22yk7j22z@macbook-pro-5.dhcp.thefacebook.com>
Date:   Sun, 20 Nov 2022 09:28:15 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     David Vernet <void@...ifault.com>
Cc:     ast@...nel.org, andrii@...nel.org, daniel@...earbox.net,
        martin.lau@...ux.dev, yhs@...com, song@...nel.org, sdf@...gle.com,
        john.fastabend@...il.com, haoluo@...gle.com, jolsa@...nel.org,
        kpsingh@...nel.org, memxor@...il.com, tj@...nel.org,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-team@...com
Subject: Re: [PATCH bpf-next v9 2/4] bpf: Allow trusted pointers to be passed
 to KF_TRUSTED_ARGS kfuncs

On Sat, Nov 19, 2022 at 11:10:02PM -0600, David Vernet wrote:
>  		case KF_ARG_PTR_TO_BTF_ID:
>  			/* Only base_type is checked, further checks are done here */
> -			if (reg->type != PTR_TO_BTF_ID &&
> -			    (!reg2btf_ids[base_type(reg->type)] || type_flag(reg->type))) {
> -				verbose(env, "arg#%d expected pointer to btf or socket\n", i);
> +			if ((base_type(reg->type) != PTR_TO_BTF_ID ||
> +			     bpf_type_has_unsafe_modifiers(reg->type)) &&
> +			    !reg2btf_ids[base_type(reg->type)]) {
> +				verbose(env, "arg#%d is %s ", i, reg_type_str(env, reg->type));
> +				verbose(env, "expected %s or socket\n",
> +					reg_type_str(env, base_type(reg->type) |
> +							  (type_flag(reg->type) & BPF_REG_TRUSTED_MODIFIERS)));
...
> diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
> index 86d6fef2e3b4..3193915c5ee6 100644
> --- a/tools/testing/selftests/bpf/verifier/calls.c
> +++ b/tools/testing/selftests/bpf/verifier/calls.c
> @@ -109,7 +109,7 @@
>  	},
>  	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
>  	.result = REJECT,
> -	.errstr = "arg#0 expected pointer to btf or socket",
> +	.errstr = "arg#0 is ptr_or_null_ expected ptr_ or socket",

Nice.
I missed the fact that reg_type_str() prints only the type.
We see more verbose prints in print_verifier_state():
  verbose(env, "%s", reg_type_str(env, t));
  if (base_type(t) == PTR_TO_BTF_ID)
          verbose(env, "%s", kernel_type_name(reg->btf, reg->btf_id));
Since reg_type_str() prints into a buffer maybe we can enhance it with
struct name printing too?
Not urgent.
The set looks great. Applied.
There is an odd arm64 failure in bonding test reported by CI, but looks unrelated.