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
