On Thu, Jul 28, 2022 at 2:02 AM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > On Thu, 28 Jul 2022 at 10:45, Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > > > On Thu, 28 Jul 2022 at 10:18, Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote: > > > > > > > From: Roberto Sassu [mailto:roberto.sassu@xxxxxxxxxx] > > > > Sent: Thursday, July 28, 2022 9:46 AM > > > > > From: Kumar Kartikeya Dwivedi [mailto:memxor@xxxxxxxxx] > > > > > Sent: Wednesday, July 27, 2022 10:16 AM > > > > > Similar to how we detect mem, size pairs in kfunc, teach verifier to > > > > > treat __ref suffix on argument name to imply that it must be a trusted > > > > > arg when passed to kfunc, similar to the effect of KF_TRUSTED_ARGS flag > > > > > but limited to the specific parameter. This is required to ensure that > > > > > kfunc that operate on some object only work on acquired pointers and not > > > > > normal PTR_TO_BTF_ID with same type which can be obtained by pointer > > > > > walking. Release functions need not specify such suffix on release > > > > > arguments as they are already expected to receive one referenced > > > > > argument. > > > > > > > > Thanks, Kumar. I will try it. > > > > > > Uhm. I realized that I was already using another suffix, > > > __maybe_null, to indicate that a caller can pass NULL as > > > argument. > > > > > > Wouldn't probably work well with two suffixes. > > > > > > > Then you can maybe extend it to parse two suffixes at most (for now atleast)? > > > > > Have you considered to extend BTF_ID_FLAGS to take five > > > extra arguments, to set flags for each kfunc parameter? > > > > > > > I didn't understand this. Flags parameter is an OR of the flags you > > set, why would we want to extend it to take 5 args? > > You can just or f1 | f2 | f3 | f4 | f5, as many as you want. > > Oh, so you mean having 5 more args to indicate flags on each > parameter? It is possible, but I think the scheme for now works ok. If > you extend it to parse two suffixes, it should be fine. Yes, the > variable name would be ugly, but you can just make a copy into a > properly named one. This is the best we can do without switching to > BTF tags. We can revisit this when we start having 4 or 5 tags on a > single parameter. > > To make it a bit less verbose you could probably call maybe_null just null? Thank you for posting the patch. It still feels that this extra flexibility gets convoluted. I'm not sure Roberto's kfunc actually needs __ref. All pointers should be pointers. Hacking -1 and -2 into a pointer is something that key infra did, but it doesn't mean that we have to carry over it into bpf kfunc.
