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. -- paul-moore.com
