linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Vernet <void@manifault.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	martin.lau@linux.dev, yhs@fb.com, song@kernel.org,
	sdf@google.com, john.fastabend@gmail.com, haoluo@google.com,
	jolsa@kernel.org, kpsingh@kernel.org, memxor@gmail.com,
	tj@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: Re: [PATCH bpf-next v9 2/4] bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs
Date: Sun, 20 Nov 2022 11:39:44 -0600	[thread overview]
Message-ID: <Y3pmYKTKhdf69MPW@maniforge.lan> (raw)
In-Reply-To: <20221120172815.godn2rt22yk7j22z@macbook-pro-5.dhcp.thefacebook.com>

On Sun, Nov 20, 2022 at 09:28:15AM -0800, Alexei Starovoitov wrote:
> 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?

I like that, and we have a bit of extra space after bumping
TYPE_STR_BUF_LEN to 128 so why not. I'll take care of it in a follow-up
change.

> Not urgent.
> The set looks great. Applied.

Thanks!

> There is an odd arm64 failure in bonding test reported by CI, but looks unrelated.

Hmmm yeah, can't see how this change would have affected that. I'll keep
an eye on it in CI to see if it persists.

  reply	other threads:[~2022-11-20 17:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-20  5:10 [PATCH bpf-next v9 0/4] Support storing struct task_struct objects as kptrs David Vernet
2022-11-20  5:10 ` [PATCH bpf-next v9 1/4] bpf: Allow multiple modifiers in reg_type_str() prefix David Vernet
2022-11-20  5:10 ` [PATCH bpf-next v9 2/4] bpf: Allow trusted pointers to be passed to KF_TRUSTED_ARGS kfuncs David Vernet
2022-11-20 17:28   ` Alexei Starovoitov
2022-11-20 17:39     ` David Vernet [this message]
2022-11-20 19:45   ` Kumar Kartikeya Dwivedi
2022-11-21 15:31     ` David Vernet
2022-11-21 16:06       ` Kumar Kartikeya Dwivedi
2022-11-20  5:10 ` [PATCH bpf-next v9 3/4] bpf: Add kfuncs for storing struct task_struct * as a kptr David Vernet
2022-12-05 10:11   ` Matus Jokay
2022-12-05 15:02     ` David Vernet
2022-11-20  5:10 ` [PATCH bpf-next v9 4/4] bpf/selftests: Add selftests for new task kfuncs David Vernet
2022-11-20 17:30 ` [PATCH bpf-next v9 0/4] Support storing struct task_struct objects as kptrs patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3pmYKTKhdf69MPW@maniforge.lan \
    --to=void@manifault.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kernel-team@fb.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=tj@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).