On 5/3/2024 8:36 AM, Paul Moore wrote: > On Fri, May 3, 2024 at 11:26 AM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: >> On 5/2/2024 5:58 PM, Paul Moore wrote: >>> The current security_inode_setxattr() and security_inode_removexattr() >>> hooks rely on individual LSMs to either call into the associated >>> capability hooks (cap_inode_setxattr() or cap_inode_removexattr()), or >>> return a magic value of 1 to indicate that the LSM layer itself should >>> perform the capability checks. Unfortunately, with the default return >>> value for these LSM hooks being 0, an individual LSM hook returning a >>> 1 will cause the LSM hook processing to exit early, potentially >>> skipping a LSM. Thankfully, with the exception of the BPF LSM, none >>> of the LSMs which currently register inode xattr hooks should end up >>> returning a value of 1, and in the BPF LSM case, with the BPF LSM hooks >>> executing last there should be no real harm in stopping processing of >>> the LSM hooks. However, the reliance on the individual LSMs to either >>> call the capability hooks themselves, or signal the LSM with a return >>> value of 1, is fragile and relies on a specific set of LSMs being >>> enabled. This patch is an effort to resolve, or minimize, these >>> issues. >>> >>> Before we discuss the solution, >> https://lore.kernel.org/lkml/20231215221636.105680-1-casey@xxxxxxxxxxxxxxxx/T/#mac61625dc1983d13ee5e8015fd22e1165381f079 >> >> ... or am I missing something? > Yes, that patch, as well as some of the others that have been posted, > change the ordering of the access control checks, moving the LSM-based > checks ahead of the capability-based checks. The patch I'm proposing > here not only preserves the current ordering, but it sticks with our > access control convention of DAC-before-LSM. Fair enough.