Hi Al,
Thanks for your quick reply.
> On Jul 25, 2024, at 10:34 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Jul 25, 2024 at 04:47:05PM -0700, Song Liu wrote:
>
>> +__bpf_kfunc struct dentry *bpf_file_dentry(const struct file *file)
>> +{
>> + /* file_dentry() does not hold reference to the dentry. We add a
>> + * dget() here so that we can add KF_ACQUIRE flag to
>> + * bpf_file_dentry().
>> + */
>> + return dget(file_dentry(file));
>> +}
>> +
>> +__bpf_kfunc struct dentry *bpf_dget_parent(struct dentry *dentry)
>> +{
>> + return dget_parent(dentry);
>> +}
>> +
>> +__bpf_kfunc void bpf_dput(struct dentry *dentry)
>> +{
>> + return dput(dentry);
>> +}
>
> If you keep a file reference, why bother grabbing dentry one?
> If not, you have a very bad trouble if that opened file is the only
> thing that keeps the filesystem busy.
Yes, we keep a file reference for the duration of the BPF program.
Therefore, it is technically not necessary to grab a dentry one.
However, we grab a dentry reference to make the dentry pointer
returned by bpf_file_dentry() a trusted pointer from BPF verifier's
POV, so that these kfuncs are more robust.
The following explanation is a bit long. Please let me know if it
turns out confusing.
==== What is trusted pointer? ====
Trusted point is the mechanism to make sure bpf kfuncs are
called with valid pointer. The BPF verifier requires certain BPF
kfuncs (helpers) are called with trusted pointers. A pointer is
trusted if one of the following two is true:
1. The pointer is passed directly by the tracepoint/kprobe, i.e.,
no pointer walking, no non-zero offset. For example,
int bpf_security_file_open(struct file *file) /* file is trusted */
{
/* mapping is not trusted */
struct address_space *mapping = file->f_mapping;
/* file2 is not trusted */
struct file *file2 = file + 1;
}
2. The pointer is returned by a kfunc with KF_ACQUIRE. This pointer
has to be released by a kfunc with KF_RELEASE. KF_ACQUIRE and
KF_RELEASE kfuncs are like any _get() _put() pairs.
==== bpf_dget_parent and bpf_dput ====
In this case, bpf_dget_parent() is a KF_ACQUIRE kfunc and
bpf_dput() is a KF_RELEASE function. They are just like regular
_get() _put() functions.
The BPF verifier makes sure pointers acquired by bpf_dget_parent()
is always released by bpf_dput() before the BPF program returns.
For example, in the following BPF program:
xxxx(struct dentry *d)
{
struct dentry *parent = bpf_dget_parent(d);
/* main logic */
bpf_dput(parent);
}
If the bpf_dput() call is missing, the verifier will not allow
the program to load.
==== More on kfunc safety ====
Trusted point makes kfunc calls safe. In this case, we want
bpf_get_dentry_xattr() to only take trusted dentry pointer.
For example, in the security_inode_listxattr LSM hook:
bpf_security_inode_listxattr(struct dentry *dentry)
{
/* This is allowed, dentry is an input and thus
* is trusted
*/
bpf_get_dentry_xattr(dentry);
/* This is not allowed, as dentry->d_parent is
* not trusted
*/
bpf_get_dentry_xattr(dentry->d_parent);
/* This is allowed, as bpf_dget_parent() holds
* a reference to d_parent, and returns a trusted
* pointer
*/
struct dentry *parent = bpf_dget_parent(dentry);
/* The following is needed, as we need the release
* parent pointer. If this line is missing, this
* program cannot pass BPF verifier.
*/
bpf_dput(parent);
}
==== bpf_file_dentry ====
In this use case, we want to get from file pointer, such as
LSM hook security_file_open() to the dentry and thus walk the
directory tree. However, security_file_open() does not pass
in a dentry pointer, and file->f_path.dentry is not a trusted
pointer. There are two ways to get a trusted dentry pointer
from a file pointer:
1. As what we do here, use bpf_file_dentry() to hold a
reference on file->f_path.dentry and return a trusted
pointer.
2. Give the verifier special knowledge that if file pointer
is trusted, file->f_path.dentry is also trusted. This
can be achieve with the following macros:
BTF_TYPE_SAFE_TRUSTED
BTF_TYPE_SAFE_RCU
BTF_TYPE_SAFE_RCU_OR_NULL.
Using the second method here requires a little more work in the
BPF verifier, as dentry is not a simple pointer in struct file,
but f_path.dentry. Therefore, I chose current approach that
bpf_file_dentry() holds a reference on dentry pointer, and the
pointer has to be released with bpf_dput().
For more details about trusted pointers in kfuncs, please refer to
Documentation/bpf/kfuncs.rst.
Does this answer your question?
Thanks,
Song
> It's almost certainly a wrong interface; please, explain what
> exactly are you trying to do here.
