Hi Christian, > On Jul 26, 2024, at 12:06 AM, Christian Brauner <brauner@xxxxxxxxxx> 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); >> + } >> +