[RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Matt Bobrowski]

Hi,

Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
have it's fixed offset set to zero, or else the BPF program being
loaded will be outright rejected by the BPF verifier.

This non-zero fixed offset restriction in most cases makes a lot of
sense, as it's considered to be a robust means of assuring that the
supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
upholds it's PTR_TRUSTED property. However, I believe that there are
also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
offset can still be considered as something which posses the
PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
hold true for selected embedded data structure members present within
given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
task_struct.thread_info, struct file.f_path.

Take for example the struct thread_info which is embedded within
struct task_struct. In a BPF program, if we happened to acquire a
PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
bpf_get_current_task_btf(), and then constructed a pointer of type
struct thread_info which was assigned the address of the embedded
struct task_struct.thread_info member, we'd have ourselves a
PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
hypothetically also say that we had a BPF kfunc that took a struct
thread_info pointer as an argument and the BPF kfunc was also
annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
kfunc, the BPF program would be rejected by the BPF verifier. This is
irrespective of the fact that supplying pointers to such embedded data
structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
to be safe.

One of the ideas that I had in mind to workaround the non-zero fixed
offset restriction was to simply introduce a new BPF kfunc annotation
i.e. __offset_allowed that could be applied on selected BPF kfunc
arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
would effectively control whether we enforce the non-zero offset
restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
and __check_ptr_off_reg(). Although, now I'm second guessing myself
and I am wondering whether introducing something like the
__offset_allowed annotation for BPF kfunc arguments could lead to
compromising any of the safety guarantees that are provided by the BPF
verifier. Does anyone see an immediate problem with using such an
approach? I raise concerns, because it feels like we're effectively
punching a hole in the BPF verifier, but it may also be perfectly safe
to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
i.e. struct thread_info, struct file, and it's just my paranoia
getting the better of me. Or, maybe someone has another idea to
support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
little more generally without the need to actually make use of any
other BPF kfunc annotations?

/M

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Yonghong Song]

On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> Hi,
>
> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> have it's fixed offset set to zero, or else the BPF program being
> loaded will be outright rejected by the BPF verifier.
>
> This non-zero fixed offset restriction in most cases makes a lot of
> sense, as it's considered to be a robust means of assuring that the
> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> upholds it's PTR_TRUSTED property. However, I believe that there are
> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> offset can still be considered as something which posses the
> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> hold true for selected embedded data structure members present within
> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> task_struct.thread_info, struct file.nf_path.
>
> Take for example the struct thread_info which is embedded within
> struct task_struct. In a BPF program, if we happened to acquire a
> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> bpf_get_current_task_btf(), and then constructed a pointer of type
> struct thread_info which was assigned the address of the embedded
> struct task_struct.thread_info member, we'd have ourselves a
> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> hypothetically also say that we had a BPF kfunc that took a struct
> thread_info pointer as an argument and the BPF kfunc was also
> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> kfunc, the BPF program would be rejected by the BPF verifier. This is
> irrespective of the fact that supplying pointers to such embedded data
> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> to be safe.
>
> One of the ideas that I had in mind to workaround the non-zero fixed
> offset restriction was to simply introduce a new BPF kfunc annotation
> i.e. __offset_allowed that could be applied on selected BPF kfunc
> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> would effectively control whether we enforce the non-zero offset
> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> and __check_ptr_off_reg(). Although, now I'm second guessing myself
> and I am wondering whether introducing something like the
> __offset_allowed annotation for BPF kfunc arguments could lead to
> compromising any of the safety guarantees that are provided by the BPF
> verifier. Does anyone see an immediate problem with using such an
> approach? I raise concerns, because it feels like we're effectively
> punching a hole in the BPF verifier, but it may also be perfectly safe
> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> i.e. struct thread_info, struct file, and it's just my paranoia
> getting the better of me. Or, maybe someone has another idea to
> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> little more generally without the need to actually make use of any
> other BPF kfunc annotations?

In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
a pointer of a particular struct is safe and trusted if the point
of that struct is trusted, e.g.,

BTF_TYPE_SAFE_TRUSTED(struct file) {
         struct inode *f_inode;
};

We do the above since gcc does not support btf_tag yet.

I guess you could do

BTF_TYPE_SAFE_TRUSTED(struct file) {
         struct path f_path;
};

and enhance verifier with the above information.

But the above 'struct path f_path' may unnecessary
consume extra memory since we only care about field
'f_path'. Maybe create a new construct like

/* pointee is a field of the struct */
BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
         struct path *f_path;
};


>
> /M
>

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Matt Bobrowski]

On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
> 
> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> > Hi,
> > 
> > Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> > supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> > have it's fixed offset set to zero, or else the BPF program being
> > loaded will be outright rejected by the BPF verifier.
> > 
> > This non-zero fixed offset restriction in most cases makes a lot of
> > sense, as it's considered to be a robust means of assuring that the
> > supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> > upholds it's PTR_TRUSTED property. However, I believe that there are
> > also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> > offset can still be considered as something which posses the
> > PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> > is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> > hold true for selected embedded data structure members present within
> > given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> > task_struct.thread_info, struct file.nf_path.
> > 
> > Take for example the struct thread_info which is embedded within
> > struct task_struct. In a BPF program, if we happened to acquire a
> > PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> > bpf_get_current_task_btf(), and then constructed a pointer of type
> > struct thread_info which was assigned the address of the embedded
> > struct task_struct.thread_info member, we'd have ourselves a
> > PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> > hypothetically also say that we had a BPF kfunc that took a struct
> > thread_info pointer as an argument and the BPF kfunc was also
> > annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> > PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> > kfunc, the BPF program would be rejected by the BPF verifier. This is
> > irrespective of the fact that supplying pointers to such embedded data
> > structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> > to be safe.
> > 
> > One of the ideas that I had in mind to workaround the non-zero fixed
> > offset restriction was to simply introduce a new BPF kfunc annotation
> > i.e. __offset_allowed that could be applied on selected BPF kfunc
> > arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> > would effectively control whether we enforce the non-zero offset
> > restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> > and __check_ptr_off_reg(). Although, now I'm second guessing myself
> > and I am wondering whether introducing something like the
> > __offset_allowed annotation for BPF kfunc arguments could lead to
> > compromising any of the safety guarantees that are provided by the BPF
> > verifier. Does anyone see an immediate problem with using such an
> > approach? I raise concerns, because it feels like we're effectively
> > punching a hole in the BPF verifier, but it may also be perfectly safe
> > to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> > i.e. struct thread_info, struct file, and it's just my paranoia
> > getting the better of me. Or, maybe someone has another idea to
> > support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> > little more generally without the need to actually make use of any
> > other BPF kfunc annotations?
> 
> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
> a pointer of a particular struct is safe and trusted if the point
> of that struct is trusted, e.g.,
> 
> BTF_TYPE_SAFE_TRUSTED(struct file) {
>         struct inode *f_inode;
> };
> 
> We do the above since gcc does not support btf_tag yet.

Yes, I'm rather familiar with this construct.

> I guess you could do
> 
> BTF_TYPE_SAFE_TRUSTED(struct file) {
>         struct path f_path;
> };
> 
> and enhance verifier with the above information.
> 
> But the above 'struct path f_path' may unnecessary
> consume extra memory since we only care about field
> 'f_path'. Maybe create a new construct like
> 
> /* pointee is a field of the struct */
> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
>         struct path *f_path;
> };

I don't fully understand how something like
BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
elaborating on that a little?

What I'm currently thinking is that with something like
BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
| PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
verifier can also check that fixed offset for the supplied
PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
explicitly annotated as being trusted via
BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
use of an __offset_allowed annotation, which would solely rely on the
btf_struct_ids_match() check for its safety.

/M

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Yonghong Song]

On 4/17/24 1:19 PM, Matt Bobrowski wrote:
> On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
>> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
>>> Hi,
>>>
>>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
>>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
>>> have it's fixed offset set to zero, or else the BPF program being
>>> loaded will be outright rejected by the BPF verifier.
>>>
>>> This non-zero fixed offset restriction in most cases makes a lot of
>>> sense, as it's considered to be a robust means of assuring that the
>>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
>>> upholds it's PTR_TRUSTED property. However, I believe that there are
>>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
>>> offset can still be considered as something which posses the
>>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
>>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
>>> hold true for selected embedded data structure members present within
>>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
>>> task_struct.thread_info, struct file.nf_path.
>>>
>>> Take for example the struct thread_info which is embedded within
>>> struct task_struct. In a BPF program, if we happened to acquire a
>>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
>>> bpf_get_current_task_btf(), and then constructed a pointer of type
>>> struct thread_info which was assigned the address of the embedded
>>> struct task_struct.thread_info member, we'd have ourselves a
>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
>>> hypothetically also say that we had a BPF kfunc that took a struct
>>> thread_info pointer as an argument and the BPF kfunc was also
>>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
>>> kfunc, the BPF program would be rejected by the BPF verifier. This is
>>> irrespective of the fact that supplying pointers to such embedded data
>>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
>>> to be safe.
>>>
>>> One of the ideas that I had in mind to workaround the non-zero fixed
>>> offset restriction was to simply introduce a new BPF kfunc annotation
>>> i.e. __offset_allowed that could be applied on selected BPF kfunc
>>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
>>> would effectively control whether we enforce the non-zero offset
>>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
>>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
>>> and I am wondering whether introducing something like the
>>> __offset_allowed annotation for BPF kfunc arguments could lead to
>>> compromising any of the safety guarantees that are provided by the BPF
>>> verifier. Does anyone see an immediate problem with using such an
>>> approach? I raise concerns, because it feels like we're effectively
>>> punching a hole in the BPF verifier, but it may also be perfectly safe
>>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
>>> i.e. struct thread_info, struct file, and it's just my paranoia
>>> getting the better of me. Or, maybe someone has another idea to
>>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
>>> little more generally without the need to actually make use of any
>>> other BPF kfunc annotations?
>> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
>> a pointer of a particular struct is safe and trusted if the point
>> of that struct is trusted, e.g.,
>>
>> BTF_TYPE_SAFE_TRUSTED(struct file) {
>>          struct inode *f_inode;
>> };
>>
>> We do the above since gcc does not support btf_tag yet.
> Yes, I'm rather familiar with this construct.
>
>> I guess you could do
>>
>> BTF_TYPE_SAFE_TRUSTED(struct file) {
>>          struct path f_path;
>> };
>>
>> and enhance verifier with the above information.
>>
>> But the above 'struct path f_path' may unnecessary
>> consume extra memory since we only care about field
>> 'f_path'. Maybe create a new construct like
>>
>> /* pointee is a field of the struct */
>> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
>>          struct path *f_path;
>> };
> I don't fully understand how something like
> BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
> elaborating on that a little?
>
> What I'm currently thinking is that with something like
> BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
> | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
> verifier can also check that fixed offset for the supplied
> PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
> explicitly annotated as being trusted via
> BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
> use of an __offset_allowed annotation, which would solely rely on the
> btf_struct_ids_match() check for its safety.
Right. What you described in the above is what I think as well.
>
> /M

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Alexei Starovoitov]

On Wed, Apr 17, 2024 at 5:11 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 4/17/24 1:19 PM, Matt Bobrowski wrote:
> > On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
> >> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> >>> Hi,
> >>>
> >>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> >>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> >>> have it's fixed offset set to zero, or else the BPF program being
> >>> loaded will be outright rejected by the BPF verifier.
> >>>
> >>> This non-zero fixed offset restriction in most cases makes a lot of
> >>> sense, as it's considered to be a robust means of assuring that the
> >>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> >>> upholds it's PTR_TRUSTED property. However, I believe that there are
> >>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> >>> offset can still be considered as something which posses the
> >>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> >>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> >>> hold true for selected embedded data structure members present within
> >>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> >>> task_struct.thread_info, struct file.nf_path.
> >>>
> >>> Take for example the struct thread_info which is embedded within
> >>> struct task_struct. In a BPF program, if we happened to acquire a
> >>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> >>> bpf_get_current_task_btf(), and then constructed a pointer of type
> >>> struct thread_info which was assigned the address of the embedded
> >>> struct task_struct.thread_info member, we'd have ourselves a
> >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> >>> hypothetically also say that we had a BPF kfunc that took a struct
> >>> thread_info pointer as an argument and the BPF kfunc was also
> >>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> >>> kfunc, the BPF program would be rejected by the BPF verifier. This is
> >>> irrespective of the fact that supplying pointers to such embedded data
> >>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> >>> to be safe.
> >>>
> >>> One of the ideas that I had in mind to workaround the non-zero fixed
> >>> offset restriction was to simply introduce a new BPF kfunc annotation
> >>> i.e. __offset_allowed that could be applied on selected BPF kfunc
> >>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> >>> would effectively control whether we enforce the non-zero offset
> >>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> >>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
> >>> and I am wondering whether introducing something like the
> >>> __offset_allowed annotation for BPF kfunc arguments could lead to
> >>> compromising any of the safety guarantees that are provided by the BPF
> >>> verifier. Does anyone see an immediate problem with using such an
> >>> approach? I raise concerns, because it feels like we're effectively
> >>> punching a hole in the BPF verifier, but it may also be perfectly safe
> >>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> >>> i.e. struct thread_info, struct file, and it's just my paranoia
> >>> getting the better of me. Or, maybe someone has another idea to
> >>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> >>> little more generally without the need to actually make use of any
> >>> other BPF kfunc annotations?
> >> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
> >> a pointer of a particular struct is safe and trusted if the point
> >> of that struct is trusted, e.g.,
> >>
> >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> >>          struct inode *f_inode;
> >> };
> >>
> >> We do the above since gcc does not support btf_tag yet.
> > Yes, I'm rather familiar with this construct.
> >
> >> I guess you could do
> >>
> >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> >>          struct path f_path;
> >> };
> >>
> >> and enhance verifier with the above information.
> >>
> >> But the above 'struct path f_path' may unnecessary
> >> consume extra memory since we only care about field
> >> 'f_path'. Maybe create a new construct like
> >>
> >> /* pointee is a field of the struct */
> >> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
> >>          struct path *f_path;
> >> };
> > I don't fully understand how something like
> > BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
> > elaborating on that a little?
> >
> > What I'm currently thinking is that with something like
> > BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
> > | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
> > verifier can also check that fixed offset for the supplied
> > PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
> > explicitly annotated as being trusted via
> > BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
> > use of an __offset_allowed annotation, which would solely rely on the
> > btf_struct_ids_match() check for its safety.
> Right. What you described in the above is what I think as well.

I believe BTF_TYPE_SAFE_* or __offset_allowed annotations
are not necessary.

In this case thread_info is the first field of struct task_struct
and I suspect the verifier already allows:

bpf_kfunc void do_stuff_with_thread(struct thread_info *ti) KF_TRUSTED_ARGS
and use it as:
task = bpf_get_current_task_btf();
do_stuff_with_thread(&task->thread_info);

We have similar setup with:
struct bpf_cpumask {
        cpumask_t cpumask;
...
};

and kfunc that accepts trusted cpumask_t * will accept
trusted struct bpf_cpumask *.
The other way around should be rejected, of course.
Similar approach should work with file/path.
The only difference is that the offset will be non-zero.

process_kf_arg_ptr_to_btf_id() needs to get smarter.

David Vernet added that check:

WARN_ON_ONCE(is_kfunc_trusted_args(meta) && reg->off);
as part of commit b613d335a743c.

iirc the reg->off==0 check is there, as an extra caution.

We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.

    struct  nf_conn___init {
            int another_field_at_off_zero;
            struct nf_conn ct;
    };

will still trigger strict_type_match as expected.

Maybe other places in the verifier need to get smarter too
to allow non-zero offset into kf_trusted_args.

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Yonghong Song]

On 4/18/24 8:03 PM, Alexei Starovoitov wrote:
> On Wed, Apr 17, 2024 at 5:11 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 4/17/24 1:19 PM, Matt Bobrowski wrote:
>>> On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
>>>> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
>>>>> Hi,
>>>>>
>>>>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
>>>>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
>>>>> have it's fixed offset set to zero, or else the BPF program being
>>>>> loaded will be outright rejected by the BPF verifier.
>>>>>
>>>>> This non-zero fixed offset restriction in most cases makes a lot of
>>>>> sense, as it's considered to be a robust means of assuring that the
>>>>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
>>>>> upholds it's PTR_TRUSTED property. However, I believe that there are
>>>>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
>>>>> offset can still be considered as something which posses the
>>>>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
>>>>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
>>>>> hold true for selected embedded data structure members present within
>>>>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
>>>>> task_struct.thread_info, struct file.nf_path.
>>>>>
>>>>> Take for example the struct thread_info which is embedded within
>>>>> struct task_struct. In a BPF program, if we happened to acquire a
>>>>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
>>>>> bpf_get_current_task_btf(), and then constructed a pointer of type
>>>>> struct thread_info which was assigned the address of the embedded
>>>>> struct task_struct.thread_info member, we'd have ourselves a
>>>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
>>>>> hypothetically also say that we had a BPF kfunc that took a struct
>>>>> thread_info pointer as an argument and the BPF kfunc was also
>>>>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
>>>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
>>>>> kfunc, the BPF program would be rejected by the BPF verifier. This is
>>>>> irrespective of the fact that supplying pointers to such embedded data
>>>>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
>>>>> to be safe.
>>>>>
>>>>> One of the ideas that I had in mind to workaround the non-zero fixed
>>>>> offset restriction was to simply introduce a new BPF kfunc annotation
>>>>> i.e. __offset_allowed that could be applied on selected BPF kfunc
>>>>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
>>>>> would effectively control whether we enforce the non-zero offset
>>>>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
>>>>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
>>>>> and I am wondering whether introducing something like the
>>>>> __offset_allowed annotation for BPF kfunc arguments could lead to
>>>>> compromising any of the safety guarantees that are provided by the BPF
>>>>> verifier. Does anyone see an immediate problem with using such an
>>>>> approach? I raise concerns, because it feels like we're effectively
>>>>> punching a hole in the BPF verifier, but it may also be perfectly safe
>>>>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
>>>>> i.e. struct thread_info, struct file, and it's just my paranoia
>>>>> getting the better of me. Or, maybe someone has another idea to
>>>>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
>>>>> little more generally without the need to actually make use of any
>>>>> other BPF kfunc annotations?
>>>> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
>>>> a pointer of a particular struct is safe and trusted if the point
>>>> of that struct is trusted, e.g.,
>>>>
>>>> BTF_TYPE_SAFE_TRUSTED(struct file) {
>>>>           struct inode *f_inode;
>>>> };
>>>>
>>>> We do the above since gcc does not support btf_tag yet.
>>> Yes, I'm rather familiar with this construct.
>>>
>>>> I guess you could do
>>>>
>>>> BTF_TYPE_SAFE_TRUSTED(struct file) {
>>>>           struct path f_path;
>>>> };
>>>>
>>>> and enhance verifier with the above information.
>>>>
>>>> But the above 'struct path f_path' may unnecessary
>>>> consume extra memory since we only care about field
>>>> 'f_path'. Maybe create a new construct like
>>>>
>>>> /* pointee is a field of the struct */
>>>> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
>>>>           struct path *f_path;
>>>> };
>>> I don't fully understand how something like
>>> BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
>>> elaborating on that a little?
>>>
>>> What I'm currently thinking is that with something like
>>> BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
>>> | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
>>> verifier can also check that fixed offset for the supplied
>>> PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
>>> explicitly annotated as being trusted via
>>> BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
>>> use of an __offset_allowed annotation, which would solely rely on the
>>> btf_struct_ids_match() check for its safety.
>> Right. What you described in the above is what I think as well.
> I believe BTF_TYPE_SAFE_* or __offset_allowed annotations
> are not necessary.
>
> In this case thread_info is the first field of struct task_struct
> and I suspect the verifier already allows:
>
> bpf_kfunc void do_stuff_with_thread(struct thread_info *ti) KF_TRUSTED_ARGS
> and use it as:
> task = bpf_get_current_task_btf();
> do_stuff_with_thread(&task->thread_info);
>
> We have similar setup with:
> struct bpf_cpumask {
>          cpumask_t cpumask;
> ...
> };
>
> and kfunc that accepts trusted cpumask_t * will accept
> trusted struct bpf_cpumask *.
> The other way around should be rejected, of course.
> Similar approach should work with file/path.
> The only difference is that the offset will be non-zero.
>
> process_kf_arg_ptr_to_btf_id() needs to get smarter.
>
> David Vernet added that check:
>
> WARN_ON_ONCE(is_kfunc_trusted_args(meta) && reg->off);
> as part of commit b613d335a743c.
>
> iirc the reg->off==0 check is there, as an extra caution.
>
> We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
>
>      struct  nf_conn___init {
>              int another_field_at_off_zero;
>              struct nf_conn ct;
>      };
>
> will still trigger strict_type_match as expected.
>
> Maybe other places in the verifier need to get smarter too
> to allow non-zero offset into kf_trusted_args.

So IIUC, for a trusted pointer, any member or nested member
access (without pointer tracing) is trusted.
For example
    struct p2 {
       struct t1 *s1;
       struct t2 s2;
    };
    struct p {
       struct p1 f1;
       struct p2 f2;
       struct p3 f3;
    }
    struct p *trust_p;

So here &trust_p->f1, &trust_p->f2, &trust_p->f3, &trust_p->f2.s2
are all trusted, right?

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Alexei Starovoitov]

On Tue, Apr 23, 2024 at 3:16 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 4/18/24 8:03 PM, Alexei Starovoitov wrote:
> > On Wed, Apr 17, 2024 at 5:11 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >>
> >> On 4/17/24 1:19 PM, Matt Bobrowski wrote:
> >>> On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
> >>>> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> >>>>> Hi,
> >>>>>
> >>>>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> >>>>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> >>>>> have it's fixed offset set to zero, or else the BPF program being
> >>>>> loaded will be outright rejected by the BPF verifier.
> >>>>>
> >>>>> This non-zero fixed offset restriction in most cases makes a lot of
> >>>>> sense, as it's considered to be a robust means of assuring that the
> >>>>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> >>>>> upholds it's PTR_TRUSTED property. However, I believe that there are
> >>>>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> >>>>> offset can still be considered as something which posses the
> >>>>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> >>>>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> >>>>> hold true for selected embedded data structure members present within
> >>>>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> >>>>> task_struct.thread_info, struct file.nf_path.
> >>>>>
> >>>>> Take for example the struct thread_info which is embedded within
> >>>>> struct task_struct. In a BPF program, if we happened to acquire a
> >>>>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> >>>>> bpf_get_current_task_btf(), and then constructed a pointer of type
> >>>>> struct thread_info which was assigned the address of the embedded
> >>>>> struct task_struct.thread_info member, we'd have ourselves a
> >>>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> >>>>> hypothetically also say that we had a BPF kfunc that took a struct
> >>>>> thread_info pointer as an argument and the BPF kfunc was also
> >>>>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> >>>>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> >>>>> kfunc, the BPF program would be rejected by the BPF verifier. This is
> >>>>> irrespective of the fact that supplying pointers to such embedded data
> >>>>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> >>>>> to be safe.
> >>>>>
> >>>>> One of the ideas that I had in mind to workaround the non-zero fixed
> >>>>> offset restriction was to simply introduce a new BPF kfunc annotation
> >>>>> i.e. __offset_allowed that could be applied on selected BPF kfunc
> >>>>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> >>>>> would effectively control whether we enforce the non-zero offset
> >>>>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> >>>>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
> >>>>> and I am wondering whether introducing something like the
> >>>>> __offset_allowed annotation for BPF kfunc arguments could lead to
> >>>>> compromising any of the safety guarantees that are provided by the BPF
> >>>>> verifier. Does anyone see an immediate problem with using such an
> >>>>> approach? I raise concerns, because it feels like we're effectively
> >>>>> punching a hole in the BPF verifier, but it may also be perfectly safe
> >>>>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> >>>>> i.e. struct thread_info, struct file, and it's just my paranoia
> >>>>> getting the better of me. Or, maybe someone has another idea to
> >>>>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> >>>>> little more generally without the need to actually make use of any
> >>>>> other BPF kfunc annotations?
> >>>> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
> >>>> a pointer of a particular struct is safe and trusted if the point
> >>>> of that struct is trusted, e.g.,
> >>>>
> >>>> BTF_TYPE_SAFE_TRUSTED(struct file) {
> >>>>           struct inode *f_inode;
> >>>> };
> >>>>
> >>>> We do the above since gcc does not support btf_tag yet.
> >>> Yes, I'm rather familiar with this construct.
> >>>
> >>>> I guess you could do
> >>>>
> >>>> BTF_TYPE_SAFE_TRUSTED(struct file) {
> >>>>           struct path f_path;
> >>>> };
> >>>>
> >>>> and enhance verifier with the above information.
> >>>>
> >>>> But the above 'struct path f_path' may unnecessary
> >>>> consume extra memory since we only care about field
> >>>> 'f_path'. Maybe create a new construct like
> >>>>
> >>>> /* pointee is a field of the struct */
> >>>> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
> >>>>           struct path *f_path;
> >>>> };
> >>> I don't fully understand how something like
> >>> BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
> >>> elaborating on that a little?
> >>>
> >>> What I'm currently thinking is that with something like
> >>> BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
> >>> | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
> >>> verifier can also check that fixed offset for the supplied
> >>> PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
> >>> explicitly annotated as being trusted via
> >>> BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
> >>> use of an __offset_allowed annotation, which would solely rely on the
> >>> btf_struct_ids_match() check for its safety.
> >> Right. What you described in the above is what I think as well.
> > I believe BTF_TYPE_SAFE_* or __offset_allowed annotations
> > are not necessary.
> >
> > In this case thread_info is the first field of struct task_struct
> > and I suspect the verifier already allows:
> >
> > bpf_kfunc void do_stuff_with_thread(struct thread_info *ti) KF_TRUSTED_ARGS
> > and use it as:
> > task = bpf_get_current_task_btf();
> > do_stuff_with_thread(&task->thread_info);
> >
> > We have similar setup with:
> > struct bpf_cpumask {
> >          cpumask_t cpumask;
> > ...
> > };
> >
> > and kfunc that accepts trusted cpumask_t * will accept
> > trusted struct bpf_cpumask *.
> > The other way around should be rejected, of course.
> > Similar approach should work with file/path.
> > The only difference is that the offset will be non-zero.
> >
> > process_kf_arg_ptr_to_btf_id() needs to get smarter.
> >
> > David Vernet added that check:
> >
> > WARN_ON_ONCE(is_kfunc_trusted_args(meta) && reg->off);
> > as part of commit b613d335a743c.
> >
> > iirc the reg->off==0 check is there, as an extra caution.
> >
> > We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> >
> >      struct  nf_conn___init {
> >              int another_field_at_off_zero;
> >              struct nf_conn ct;
> >      };
> >
> > will still trigger strict_type_match as expected.
> >
> > Maybe other places in the verifier need to get smarter too
> > to allow non-zero offset into kf_trusted_args.
>
> So IIUC, for a trusted pointer, any member or nested member
> access (without pointer tracing) is trusted.
> For example
>     struct p2 {
>        struct t1 *s1;
>        struct t2 s2;
>     };
>     struct p {
>        struct p1 f1;
>        struct p2 f2;
>        struct p3 f3;
>     }
>     struct p *trust_p;
>
> So here &trust_p->f1, &trust_p->f2, &trust_p->f3, &trust_p->f2.s2
> are all trusted, right?

Correct, but the verifier currently only allows to pass &trust_p->f1
as an argument.

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 10264 bytes --]

On Thu, Apr 18, 2024 at 08:03:40PM -0700, Alexei Starovoitov wrote:
> On Wed, Apr 17, 2024 at 5:11 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> >
> >
> > On 4/17/24 1:19 PM, Matt Bobrowski wrote:
> > > On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
> > >> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> > >>> Hi,
> > >>>
> > >>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> > >>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> > >>> have it's fixed offset set to zero, or else the BPF program being
> > >>> loaded will be outright rejected by the BPF verifier.
> > >>>
> > >>> This non-zero fixed offset restriction in most cases makes a lot of
> > >>> sense, as it's considered to be a robust means of assuring that the
> > >>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> > >>> upholds it's PTR_TRUSTED property. However, I believe that there are
> > >>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> > >>> offset can still be considered as something which posses the
> > >>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> > >>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> > >>> hold true for selected embedded data structure members present within
> > >>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> > >>> task_struct.thread_info, struct file.nf_path.
> > >>>
> > >>> Take for example the struct thread_info which is embedded within
> > >>> struct task_struct. In a BPF program, if we happened to acquire a
> > >>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> > >>> bpf_get_current_task_btf(), and then constructed a pointer of type
> > >>> struct thread_info which was assigned the address of the embedded
> > >>> struct task_struct.thread_info member, we'd have ourselves a
> > >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> > >>> hypothetically also say that we had a BPF kfunc that took a struct
> > >>> thread_info pointer as an argument and the BPF kfunc was also
> > >>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> > >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> > >>> kfunc, the BPF program would be rejected by the BPF verifier. This is
> > >>> irrespective of the fact that supplying pointers to such embedded data
> > >>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> > >>> to be safe.
> > >>>
> > >>> One of the ideas that I had in mind to workaround the non-zero fixed
> > >>> offset restriction was to simply introduce a new BPF kfunc annotation
> > >>> i.e. __offset_allowed that could be applied on selected BPF kfunc
> > >>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> > >>> would effectively control whether we enforce the non-zero offset
> > >>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> > >>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
> > >>> and I am wondering whether introducing something like the
> > >>> __offset_allowed annotation for BPF kfunc arguments could lead to
> > >>> compromising any of the safety guarantees that are provided by the BPF
> > >>> verifier. Does anyone see an immediate problem with using such an
> > >>> approach? I raise concerns, because it feels like we're effectively
> > >>> punching a hole in the BPF verifier, but it may also be perfectly safe
> > >>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> > >>> i.e. struct thread_info, struct file, and it's just my paranoia
> > >>> getting the better of me. Or, maybe someone has another idea to
> > >>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> > >>> little more generally without the need to actually make use of any
> > >>> other BPF kfunc annotations?
> > >> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
> > >> a pointer of a particular struct is safe and trusted if the point
> > >> of that struct is trusted, e.g.,
> > >>
> > >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> > >>          struct inode *f_inode;
> > >> };
> > >>
> > >> We do the above since gcc does not support btf_tag yet.
> > > Yes, I'm rather familiar with this construct.
> > >
> > >> I guess you could do
> > >>
> > >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> > >>          struct path f_path;
> > >> };
> > >>
> > >> and enhance verifier with the above information.
> > >>
> > >> But the above 'struct path f_path' may unnecessary
> > >> consume extra memory since we only care about field
> > >> 'f_path'. Maybe create a new construct like
> > >>
> > >> /* pointee is a field of the struct */
> > >> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
> > >>          struct path *f_path;
> > >> };
> > > I don't fully understand how something like
> > > BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
> > > elaborating on that a little?
> > >
> > > What I'm currently thinking is that with something like
> > > BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
> > > | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
> > > verifier can also check that fixed offset for the supplied
> > > PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
> > > explicitly annotated as being trusted via
> > > BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
> > > use of an __offset_allowed annotation, which would solely rely on the
> > > btf_struct_ids_match() check for its safety.
> > Right. What you described in the above is what I think as well.
> 
> I believe BTF_TYPE_SAFE_* or __offset_allowed annotations
> are not necessary.
> 
> In this case thread_info is the first field of struct task_struct
> and I suspect the verifier already allows:
> 
> bpf_kfunc void do_stuff_with_thread(struct thread_info *ti) KF_TRUSTED_ARGS
> and use it as:
> task = bpf_get_current_task_btf();
> do_stuff_with_thread(&task->thread_info);

Yes, I believe this should already work. It would be the same as casting
the task as a struct thread_info. btf_struct_ids_match() should
btf_struct_walk() the task, and find a struct thread_info object at that
offset and successfully compare the BTF IDs of that with the arg type.
If not for the check_func_arg_reg_off() code described below, it should
also work with nonzero offsets as well. When we begin the walk it can be
at any offset. After we do the first struct walk, we continue descending
at offset 0 from that first inner struct type.

> We have similar setup with:
> struct bpf_cpumask {
>         cpumask_t cpumask;
> ...
> };
> 
> and kfunc that accepts trusted cpumask_t * will accept
> trusted struct bpf_cpumask *.
> The other way around should be rejected, of course.
> Similar approach should work with file/path.
> The only difference is that the offset will be non-zero.

Agreed

> process_kf_arg_ptr_to_btf_id() needs to get smarter.
> 
> David Vernet added that check:
> 
> WARN_ON_ONCE(is_kfunc_trusted_args(meta) && reg->off);
> as part of commit b613d335a743c.
> 
> iirc the reg->off==0 check is there, as an extra caution.

That check is currently an invariant because of this code:

11720                 case KF_ARG_PTR_TO_MAP:
11721                 case KF_ARG_PTR_TO_ALLOC_BTF_ID:
11722                 case KF_ARG_PTR_TO_BTF_ID:
11723                         if (!is_kfunc_trusted_args(meta) && !is_kfunc_rcu(meta))
11724                                 break;
11725
11726                         if (!is_trusted_reg(reg)) {
11727                                 if (!is_kfunc_rcu(meta)) {
11728                                         verbose(env, "R%d must be referenced or trusted\n", regno);
11729                                         return -EINVAL;
11730                                 }
11731                                 if (!is_rcu_reg(reg)) {
11732                                         verbose(env, "R%d must be a rcu pointer\n", regno);
11733                                         return -EINVAL;
11734                                 }
11735                         }
11736
11737                         fallthrough;
11738                 case KF_ARG_PTR_TO_CTX:
11739                         /* Trusted arguments have the same offset checks as release arguments */
11740                         arg_type |= OBJ_RELEASE;  <<<<< because of this
11741                         break;

The OBJ_RELEASE causes check_func_arg_reg_off() to fail to verify if
there's a nonzero offset. In reality, I _think_ we only need to check
for a nonzero offset for KF_RELEASE, and possibly KF_ACQUIRE.

> 
> We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> 
>     struct  nf_conn___init {
>             int another_field_at_off_zero;
>             struct nf_conn ct;
>     };
> 
> will still trigger strict_type_match as expected.

Yes, this should continue to just work, but I think we may also have to
be cognizant to not allow this type of pattern:

struct some_other_type {
	int field_at_off_zero;
	struct nf_conn___init ct;
};

In this case, we don't want to allow &other_type->ct to be passed to a
kfunc expecting a struct nf_conn. So we'd also have to compare the type
at the register offset to make sure it's not a nocast alias, not just
the type in the register itself. I'm not sure if this is a problem in
practice. I expect it isn't. struct nf_conn___init exists solely to
allow the struct nf_conn kfuncs to enforce calling semantics so that an
uninitialized struct nf_conn object can't be passed to specific kfuncs
that are expecting an initialized object. I don't see why we'd ever
embed a wrapper type like that inside of another type. But still
something to be cognizant of.

> Maybe other places in the verifier need to get smarter too
> to allow non-zero offset into kf_trusted_args.

Yes, see above. We're also validating that trusted args have a zero
offset in check_func_arg_reg_off(). There may be other places too, but I
don't think there are too many.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Alexei Starovoitov]

On Tue, Apr 23, 2024 at 10:50 PM David Vernet <void@manifault.com> wrote:
>
> On Thu, Apr 18, 2024 at 08:03:40PM -0700, Alexei Starovoitov wrote:
> > On Wed, Apr 17, 2024 at 5:11 PM Yonghong Song <yonghong.song@linux.dev> wrote:
> > >
> > >
> > > On 4/17/24 1:19 PM, Matt Bobrowski wrote:
> > > > On Mon, Apr 15, 2024 at 09:43:42AM -0700, Yonghong Song wrote:
> > > >> On 4/12/24 4:31 AM, Matt Bobrowski wrote:
> > > >>> Hi,
> > > >>>
> > > >>> Currently, if a BPF kfunc has been annotated with KF_TRUSTED_ARGS, any
> > > >>> supplied PTR_TO_BTF_ID | PTR_TRUSTED argument to that BPF kfunc must
> > > >>> have it's fixed offset set to zero, or else the BPF program being
> > > >>> loaded will be outright rejected by the BPF verifier.
> > > >>>
> > > >>> This non-zero fixed offset restriction in most cases makes a lot of
> > > >>> sense, as it's considered to be a robust means of assuring that the
> > > >>> supplied PTR_TO_BTF_ID to the KF_TRUSTED_ARGS annotated BPF kfunc
> > > >>> upholds it's PTR_TRUSTED property. However, I believe that there are
> > > >>> also cases out there whereby a PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed
> > > >>> offset can still be considered as something which posses the
> > > >>> PTR_TRUSTED property, and could be safely passed to a BPF kfunc that
> > > >>> is annotated w/ KF_TRUSTED_ARGS. I believe that this can particularly
> > > >>> hold true for selected embedded data structure members present within
> > > >>> given PTR_TO_BTF_ID | PTR_TRUSTED types i.e. struct
> > > >>> task_struct.thread_info, struct file.nf_path.
> > > >>>
> > > >>> Take for example the struct thread_info which is embedded within
> > > >>> struct task_struct. In a BPF program, if we happened to acquire a
> > > >>> PTR_TO_BTF_ID | PTR_TRUSTED for a struct task_struct via
> > > >>> bpf_get_current_task_btf(), and then constructed a pointer of type
> > > >>> struct thread_info which was assigned the address of the embedded
> > > >>> struct task_struct.thread_info member, we'd have ourselves a
> > > >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ a fixed offset. Now, let's
> > > >>> hypothetically also say that we had a BPF kfunc that took a struct
> > > >>> thread_info pointer as an argument and the BPF kfunc was also
> > > >>> annotated w/ KF_TRUSTED_ARGS. If we attempted to pass the constructed
> > > >>> PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset to this hypothetical BPF
> > > >>> kfunc, the BPF program would be rejected by the BPF verifier. This is
> > > >>> irrespective of the fact that supplying pointers to such embedded data
> > > >>> structure members of a PTR_TO_BTF_ID | PTR_TRUSTED may be considered
> > > >>> to be safe.
> > > >>>
> > > >>> One of the ideas that I had in mind to workaround the non-zero fixed
> > > >>> offset restriction was to simply introduce a new BPF kfunc annotation
> > > >>> i.e. __offset_allowed that could be applied on selected BPF kfunc
> > > >>> arguments that are expected to be KF_TRUSTED_ARGS. Such an annotation
> > > >>> would effectively control whether we enforce the non-zero offset
> > > >>> restriction or not in check_kfunc_args(), check_func_arg_reg_off(),
> > > >>> and __check_ptr_off_reg(). Although, now I'm second guessing myself
> > > >>> and I am wondering whether introducing something like the
> > > >>> __offset_allowed annotation for BPF kfunc arguments could lead to
> > > >>> compromising any of the safety guarantees that are provided by the BPF
> > > >>> verifier. Does anyone see an immediate problem with using such an
> > > >>> approach? I raise concerns, because it feels like we're effectively
> > > >>> punching a hole in the BPF verifier, but it may also be perfectly safe
> > > >>> to do on carefully selected PTR_TO_BTF_ID | PTR_TRUSTED types
> > > >>> i.e. struct thread_info, struct file, and it's just my paranoia
> > > >>> getting the better of me. Or, maybe someone has another idea to
> > > >>> support PTR_TO_BTF_ID | PTR_TRUSTED w/ fixed offset safely and a
> > > >>> little more generally without the need to actually make use of any
> > > >>> other BPF kfunc annotations?
> > > >> In verifier.c, we have BTF_TYPE_SAFE_TRUSTED to indidate that
> > > >> a pointer of a particular struct is safe and trusted if the point
> > > >> of that struct is trusted, e.g.,
> > > >>
> > > >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> > > >>          struct inode *f_inode;
> > > >> };
> > > >>
> > > >> We do the above since gcc does not support btf_tag yet.
> > > > Yes, I'm rather familiar with this construct.
> > > >
> > > >> I guess you could do
> > > >>
> > > >> BTF_TYPE_SAFE_TRUSTED(struct file) {
> > > >>          struct path f_path;
> > > >> };
> > > >>
> > > >> and enhance verifier with the above information.
> > > >>
> > > >> But the above 'struct path f_path' may unnecessary
> > > >> consume extra memory since we only care about field
> > > >> 'f_path'. Maybe create a new construct like
> > > >>
> > > >> /* pointee is a field of the struct */
> > > >> BTF_TYPE_SAFE_FIELD_TRUSTED(struct file) {
> > > >>          struct path *f_path;
> > > >> };
> > > > I don't fully understand how something like
> > > > BTF_TYPE_SAFE_FIELD_TRUSTED could work in practice. Do you mind
> > > > elaborating on that a little?
> > > >
> > > > What I'm currently thinking is that with something like
> > > > BTF_TYPE_SAFE_FIELD_TRUSTED, if the BPF verifier sees a PTR_TO_BTF_ID
> > > > | PTR_TRUSTED w/ a fixed offset supplied to a BPF kfunc, then the BPF
> > > > verifier can also check that fixed offset for the supplied
> > > > PTR_TO_BTF_ID | PTR_TRUSTED actually accesses a member that has been
> > > > explicitly annotated as being trusted via
> > > > BTF_TYPE_SAFE_FIELD_TRUSTED. Maybe that would be better then making
> > > > use of an __offset_allowed annotation, which would solely rely on the
> > > > btf_struct_ids_match() check for its safety.
> > > Right. What you described in the above is what I think as well.
> >
> > I believe BTF_TYPE_SAFE_* or __offset_allowed annotations
> > are not necessary.
> >
> > In this case thread_info is the first field of struct task_struct
> > and I suspect the verifier already allows:
> >
> > bpf_kfunc void do_stuff_with_thread(struct thread_info *ti) KF_TRUSTED_ARGS
> > and use it as:
> > task = bpf_get_current_task_btf();
> > do_stuff_with_thread(&task->thread_info);
>
> Yes, I believe this should already work. It would be the same as casting
> the task as a struct thread_info. btf_struct_ids_match() should
> btf_struct_walk() the task, and find a struct thread_info object at that
> offset and successfully compare the BTF IDs of that with the arg type.
> If not for the check_func_arg_reg_off() code described below, it should
> also work with nonzero offsets as well. When we begin the walk it can be
> at any offset. After we do the first struct walk, we continue descending
> at offset 0 from that first inner struct type.
>
> > We have similar setup with:
> > struct bpf_cpumask {
> >         cpumask_t cpumask;
> > ...
> > };
> >
> > and kfunc that accepts trusted cpumask_t * will accept
> > trusted struct bpf_cpumask *.
> > The other way around should be rejected, of course.
> > Similar approach should work with file/path.
> > The only difference is that the offset will be non-zero.
>
> Agreed
>
> > process_kf_arg_ptr_to_btf_id() needs to get smarter.
> >
> > David Vernet added that check:
> >
> > WARN_ON_ONCE(is_kfunc_trusted_args(meta) && reg->off);
> > as part of commit b613d335a743c.
> >
> > iirc the reg->off==0 check is there, as an extra caution.
>
> That check is currently an invariant because of this code:
>
> 11720                 case KF_ARG_PTR_TO_MAP:
> 11721                 case KF_ARG_PTR_TO_ALLOC_BTF_ID:
> 11722                 case KF_ARG_PTR_TO_BTF_ID:
> 11723                         if (!is_kfunc_trusted_args(meta) && !is_kfunc_rcu(meta))
> 11724                                 break;
> 11725
> 11726                         if (!is_trusted_reg(reg)) {
> 11727                                 if (!is_kfunc_rcu(meta)) {
> 11728                                         verbose(env, "R%d must be referenced or trusted\n", regno);
> 11729                                         return -EINVAL;
> 11730                                 }
> 11731                                 if (!is_rcu_reg(reg)) {
> 11732                                         verbose(env, "R%d must be a rcu pointer\n", regno);
> 11733                                         return -EINVAL;
> 11734                                 }
> 11735                         }
> 11736
> 11737                         fallthrough;
> 11738                 case KF_ARG_PTR_TO_CTX:
> 11739                         /* Trusted arguments have the same offset checks as release arguments */
> 11740                         arg_type |= OBJ_RELEASE;  <<<<< because of this
> 11741                         break;
>
> The OBJ_RELEASE causes check_func_arg_reg_off() to fail to verify if
> there's a nonzero offset. In reality, I _think_ we only need to check
> for a nonzero offset for KF_RELEASE, and possibly KF_ACQUIRE.

Why special case KF_RELEASE/ACQUIRE ?
I think they're no different from kfuncs with KF_TRUSTED_ARGS.
Should be safe to allow non-zero offset trusted arg in all cases.

> >
> > We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> >
> >     struct  nf_conn___init {
> >             int another_field_at_off_zero;
> >             struct nf_conn ct;
> >     };
> >
> > will still trigger strict_type_match as expected.
>
> Yes, this should continue to just work, but I think we may also have to
> be cognizant to not allow this type of pattern:
>
> struct some_other_type {
>         int field_at_off_zero;
>         struct nf_conn___init ct;
> };
>
> In this case, we don't want to allow &other_type->ct to be passed to a
> kfunc expecting a struct nf_conn. So we'd also have to compare the type
> at the register offset to make sure it's not a nocast alias, not just
> the type in the register itself. I'm not sure if this is a problem in
> practice. I expect it isn't. struct nf_conn___init exists solely to
> allow the struct nf_conn kfuncs to enforce calling semantics so that an
> uninitialized struct nf_conn object can't be passed to specific kfuncs
> that are expecting an initialized object. I don't see why we'd ever
> embed a wrapper type like that inside of another type. But still
> something to be cognizant of.

Agree that it's not a problem now and I wouldn't proactively
complicate the verifier.
__init types are in the kernel code and it gets code reviewed.
So 'struct some_other_type' won't happen out of nowhere.

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

On Wed, Apr 24, 2024 at 11:36:51AM -0700, Alexei Starovoitov wrote:

[...]

> > The OBJ_RELEASE causes check_func_arg_reg_off() to fail to verify if
> > there's a nonzero offset. In reality, I _think_ we only need to check
> > for a nonzero offset for KF_RELEASE, and possibly KF_ACQUIRE.
> 
> Why special case KF_RELEASE/ACQUIRE ?
> I think they're no different from kfuncs with KF_TRUSTED_ARGS.
> Should be safe to allow non-zero offset trusted arg in all cases.

Yeah, after thinking about this some more I agree with you. All we need
to do is verify that the object at the non-zero offset has a
ref_obj_id > 0 if being passed to KF_RELEASE. No different than at
offset 0. This will be a nice usability improvement. The offset=0
restriction really does seem pointless and arbitrary, unless I'm
completely missing something.

> > > We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> > >
> > >     struct  nf_conn___init {
> > >             int another_field_at_off_zero;
> > >             struct nf_conn ct;
> > >     };
> > >
> > > will still trigger strict_type_match as expected.
> >
> > Yes, this should continue to just work, but I think we may also have to
> > be cognizant to not allow this type of pattern:
> >
> > struct some_other_type {
> >         int field_at_off_zero;
> >         struct nf_conn___init ct;
> > };
> >
> > In this case, we don't want to allow &other_type->ct to be passed to a
> > kfunc expecting a struct nf_conn. So we'd also have to compare the type
> > at the register offset to make sure it's not a nocast alias, not just
> > the type in the register itself. I'm not sure if this is a problem in
> > practice. I expect it isn't. struct nf_conn___init exists solely to
> > allow the struct nf_conn kfuncs to enforce calling semantics so that an
> > uninitialized struct nf_conn object can't be passed to specific kfuncs
> > that are expecting an initialized object. I don't see why we'd ever
> > embed a wrapper type like that inside of another type. But still
> > something to be cognizant of.
> 
> Agree that it's not a problem now and I wouldn't proactively
> complicate the verifier.  __init types are in the kernel code and it
> gets code reviewed.  So 'struct some_other_type' won't happen out of
> nowhere.

Makes sense

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Kumar Kartikeya Dwivedi]

On Thu, 25 Apr 2024 at 17:59, David Vernet <void@manifault.com> wrote:
>
> On Wed, Apr 24, 2024 at 11:36:51AM -0700, Alexei Starovoitov wrote:
>
> [...]
>
> > > The OBJ_RELEASE causes check_func_arg_reg_off() to fail to verify if
> > > there's a nonzero offset. In reality, I _think_ we only need to check
> > > for a nonzero offset for KF_RELEASE, and possibly KF_ACQUIRE.
> >
> > Why special case KF_RELEASE/ACQUIRE ?
> > I think they're no different from kfuncs with KF_TRUSTED_ARGS.
> > Should be safe to allow non-zero offset trusted arg in all cases.
>
> Yeah, after thinking about this some more I agree with you. All we need
> to do is verify that the object at the non-zero offset has a
> ref_obj_id > 0 if being passed to KF_RELEASE. No different than at
> offset 0. This will be a nice usability improvement. The offset=0
> restriction really does seem pointless and arbitrary, unless I'm
> completely missing something.
>

It will be mostly ok, especially if a type match (not just type == X,
but for PTR_TO_BTF_ID, btf_struct_ids_match) follows the ref_obj_id
check (regardless of the offset).
Just be careful when such type match won't happen, e.g. with kfuncs
like bpf_obj_drop (taking a void *). In such a case off == 0 is
necessary, otherwise we'll pass the wrong pointer to the free function
in the kfunc.

We've had bugs in absence of type matching with offset != 0, e.g.
64620e0a1e71 ("bpf: Fix out of bounds access for ringbuf helpers")
comes to mind, which slipped through.

> > > > We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> > > >
> > > >     struct  nf_conn___init {
> > > >             int another_field_at_off_zero;
> > > >             struct nf_conn ct;
> > > >     };
> > > >
> > > > will still trigger strict_type_match as expected.
> > >
> > > Yes, this should continue to just work, but I think we may also have to
> > > be cognizant to not allow this type of pattern:
> > >
> > > struct some_other_type {
> > >         int field_at_off_zero;
> > >         struct nf_conn___init ct;
> > > };
> > >
> > > In this case, we don't want to allow &other_type->ct to be passed to a
> > > kfunc expecting a struct nf_conn. So we'd also have to compare the type
> > > at the register offset to make sure it's not a nocast alias, not just
> > > the type in the register itself. I'm not sure if this is a problem in
> > > practice. I expect it isn't. struct nf_conn___init exists solely to
> > > allow the struct nf_conn kfuncs to enforce calling semantics so that an
> > > uninitialized struct nf_conn object can't be passed to specific kfuncs
> > > that are expecting an initialized object. I don't see why we'd ever
> > > embed a wrapper type like that inside of another type. But still
> > > something to be cognizant of.
> >
> > Agree that it's not a problem now and I wouldn't proactively
> > complicate the verifier.  __init types are in the kernel code and it
> > gets code reviewed.  So 'struct some_other_type' won't happen out of
> > nowhere.
>
> Makes sense

Re: [RFC] bpf: allowing PTR_TO_BTF_ID | PTR_TRUSTED w/ non-zero fixed offset to selected KF_TRUSTED_ARGS BPF kfuncs

[Matt Bobrowski]

Apologies about the delayed response on this thread, I've been on
parental leave for the last couple of weeks and only just catching up
on everything now.

On Thu, Apr 25, 2024 at 06:06:00PM +0200, Kumar Kartikeya Dwivedi wrote:
> On Thu, 25 Apr 2024 at 17:59, David Vernet <void@manifault.com> wrote:
> >
> > On Wed, Apr 24, 2024 at 11:36:51AM -0700, Alexei Starovoitov wrote:
> >
> > [...]
> >
> > > > The OBJ_RELEASE causes check_func_arg_reg_off() to fail to verify if
> > > > there's a nonzero offset. In reality, I _think_ we only need to check
> > > > for a nonzero offset for KF_RELEASE, and possibly KF_ACQUIRE.
> > >
> > > Why special case KF_RELEASE/ACQUIRE ?
> > > I think they're no different from kfuncs with KF_TRUSTED_ARGS.
> > > Should be safe to allow non-zero offset trusted arg in all cases.
> >
> > Yeah, after thinking about this some more I agree with you. All we need
> > to do is verify that the object at the non-zero offset has a
> > ref_obj_id > 0 if being passed to KF_RELEASE. No different than at
> > offset 0. This will be a nice usability improvement. The offset=0
> > restriction really does seem pointless and arbitrary, unless I'm
> > completely missing something.
> >
> 
> It will be mostly ok, especially if a type match (not just type == X,
> but for PTR_TO_BTF_ID, btf_struct_ids_match) follows the ref_obj_id
> check (regardless of the offset).
> Just be careful when such type match won't happen, e.g. with kfuncs
> like bpf_obj_drop (taking a void *). In such a case off == 0 is
> necessary, otherwise we'll pass the wrong pointer to the free function
> in the kfunc.
> 
> We've had bugs in absence of type matching with offset != 0, e.g.
> 64620e0a1e71 ("bpf: Fix out of bounds access for ringbuf helpers")
> comes to mind, which slipped through.
> 
> > > > > We can allow off!=0 and it won't confuse btf_type_ids_nocast_alias.
> > > > >
> > > > >     struct  nf_conn___init {
> > > > >             int another_field_at_off_zero;
> > > > >             struct nf_conn ct;
> > > > >     };
> > > > >
> > > > > will still trigger strict_type_match as expected.
> > > >
> > > > Yes, this should continue to just work, but I think we may also have to
> > > > be cognizant to not allow this type of pattern:
> > > >
> > > > struct some_other_type {
> > > >         int field_at_off_zero;
> > > >         struct nf_conn___init ct;
> > > > };
> > > >
> > > > In this case, we don't want to allow &other_type->ct to be passed to a
> > > > kfunc expecting a struct nf_conn. So we'd also have to compare the type
> > > > at the register offset to make sure it's not a nocast alias, not just
> > > > the type in the register itself. I'm not sure if this is a problem in
> > > > practice. I expect it isn't. struct nf_conn___init exists solely to
> > > > allow the struct nf_conn kfuncs to enforce calling semantics so that an
> > > > uninitialized struct nf_conn object can't be passed to specific kfuncs
> > > > that are expecting an initialized object. I don't see why we'd ever
> > > > embed a wrapper type like that inside of another type. But still
> > > > something to be cognizant of.
> > >
> > > Agree that it's not a problem now and I wouldn't proactively
> > > complicate the verifier.  __init types are in the kernel code and it
> > > gets code reviewed.  So 'struct some_other_type' won't happen out of
> > > nowhere.
> >
> > Makes sense

OK, so based on what has been discussed within this thread, I've
understood that we're OK with generally relaxing the reg->off
restriction that is currently in place for
KF_ACQUIRE/KF_RELEASE/KF_TRUSTED_ARGS/KF_RCU flagged BPF kfuncs?

This is providing that we substitute the current !reg->off checks with
the following instead moving forward:

* For arguments passed to BPF kfuncs that makes use of the
  KF_RELEASE/KF_TRUSTED_ARGS/KF_RCU flag, ensure that the backing reg
  has a reg->ref_obj_id > 0, irrespective of the value held by
  reg->off.

* Perform a type match check against reg and the arguments of the
  KF_RELEASE/KF_TRUSTED_ARGS/KF_RCU based BPF kfunc, following the
  preceding reg->ref_obj_id > 0 check.

* If we fail to type match, because the
  KF_RELEASE/KF_TRUSTED_ARGS/KF_RCU based BPF kfunc takes a void
  pointer, fallback to enforcing the !reg->off that was previously in
  place.

Please do correct me if I've misunderstood something.

/M