linux-kernel - Re: [PATCH bpf-next 2/2] selftests/bpf: Add tests for bpf_get_dentry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1A0AAD8C-366E-45E2-A386-B4CCB5401D81@fb.com>
Date: Fri, 26 Jul 2024 09:19:54 +0000
From: Song Liu <songliubraving@...a.com>
To: Christian Brauner <brauner@...nel.org>
CC: Song Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        Linux-Fsdevel
	<linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Kernel
 Team <kernel-team@...a.com>,
        "andrii@...nel.org" <andrii@...nel.org>,
        "eddyz87@...il.com" <eddyz87@...il.com>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "martin.lau@...ux.dev"
	<martin.lau@...ux.dev>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "jack@...e.cz" <jack@...e.cz>,
        "kpsingh@...nel.org" <kpsingh@...nel.org>,
        "mattbobrowski@...gle.com" <mattbobrowski@...gle.com>
Subject: Re: [PATCH bpf-next 2/2] selftests/bpf: Add tests for
 bpf_get_dentry_xattr

Hi Christian, 

> On Jul 26, 2024, at 12:06 AM, Christian Brauner <brauner@...nel.org> wrote:

[...]

>> +
>> + for (i = 0; i < 10; i++) {
>> + ret = bpf_get_dentry_xattr(dentry, "user.kfunc", &value_ptr);
>> + if (ret == sizeof(expected_value) &&
>> +    !bpf_strncmp(value, ret, expected_value))
>> + matches++;
>> +
>> + prev_dentry = dentry;
>> + dentry = bpf_dget_parent(prev_dentry);
> 
> Why do you need to walk upwards and instead of reading the xattr values
> during security_inode_permission()?

In this use case, we would like to add xattr to the directory to cover
all files under it. For example, assume we have the following xattrs:

  /bin  xattr: user.policy_A = value_A
  /bin/gcc-6.9/ xattr: user.policy_A = value_B
  /bin/gcc-6.9/gcc xattr: user.policy_A = value_C

/bin/gcc-6.9/gcc will use value_C;
/bin/gcc-6.9/<other_files> will use value_B;
/bin/<other_folder_or_file> will use value_A;

By walking upwards from security_file_open(), we can finish the logic 
in a single LSM hook:

    repeat:
        if (dentry have user.policy_A) {
            /* make decision based on value */;
        } else {
            dentry = bpf_dget_parent();
            goto repeat;
        }

Does this make sense? Or maybe I misunderstood the suggestion?

Also, we don't have a bpf_get_inode_xattr() yet. I guess we will need
it for the security_inode_permission approach. If we agree that's a 
better approach, I more than happy to implement it that way. In fact,
I think we will eventually need both bpf_get_inode_xattr() and 
bpf_get_dentry_xattr(). 

Thanks,
Song


>> + bpf_dput(prev_dentry);
>> + }
>> +