Re: [PATCH bpf-next 2/2] selftests/bpf: Add tests for bpf_get_dentry_xattr
From: Song Liu
Date: Tue Aug 20 2024 - 03:24:43 EST
> On Aug 19, 2024, at 11:29 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Aug 19, 2024 at 08:25:38PM +0000, Song Liu wrote:
>
>> int bpf_get_parent_path(struct path *p) {
>> again:
>> if (p->dentry == p->mnt.mnt_root) {
>> follow_up(p);
>> goto again;
>> }
>> if (unlikely(IS_ROOT(p->dentry))) {
>> return PARENT_WALK_DONE;
>> }
>> parent_dentry = dget_parent(p->dentry);
>> dput(p->dentry);
>> p->dentry = parent_dentry;
>> return PARENT_WALK_NEXT;
>> }
>>
>> This will handle the mount. However, we cannot guarantee deny-by-default
>> policies like LandLock does, because this is just a building block of
>> some security policies.
>
> You do realize that above is racy as hell, right?
>
> Filesystem objects do get moved around. You can, theoretically, play with
> rename_lock, but that is highly antisocial.
I do understand filesystem objects may get moved around. However, I am not
sure whether we have to avoid all the race conditions (and whether it is
really possible to avoid all race conditions).
> What's more, _mounts_ can get moved around. That is to say, there is no
> such thing as stable canonical pathname of a file.
Maybe I should really step back and ask for high level suggestions.
We are hoping to tag all files in a directory with xattr (or something
else) on the directory. For example, a xattr "Do_not_rename" on /usr
should block rename of all files inside /usr.
Our original idea is to start from security_file_open() hook, and walk
up the tree (/usr/bin/gcc => /usr/bin => /usr). However, this appears
to be wasteful and unreliable, and Christian suggested we should use a
combination of security_inode_permission and security_file_open. I
tried to build something on this direction, and hits a few issues:
1. Getting xattr from security_inode_permission() is not easy. Some
FS requires dentry to get xattr.
2. Finding parent from security_inode_permission() is also tricky.
(maybe as trick as doing dget_parent() from security_file_open?)
We need tag on /usr to work on /usr/bin. But how do we know /usr
is /usr/bin's parent?
For the original goal… tag all files in a directory with xattr on
the directory, is it possible at all? If not, we will go back and
implement something to tag all the files one at a time. If it is
indeed possible, what's the right way to do it, and what are the
race conditions we need to avoid and to accept?
Comments and suggestions are highly appreciated.
Thanks,
Song