On Tue, Jan 30, 2024 at 9:19 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > I'll come back to this tomorrow with some fresh eyes. My apologies, "tomorrow" turned into "the day after tomorrow" (as it often does) ... I've been struggling with the idea that there are individual LSMs still calling into the capability hooks instead of leveraging the LSM stacking infrastructure, and the "magic" involved to make it all work. While your patch looks like it should restore proper behavior - that's good! - I keep thinking that we can, and should, do better. The only thing that I coming up with is to create two new LSM hooks, in addition to the existing 'inode_setxattr' hook. The new LSM hooks would be 'inode_setxattr_owned' and 'inode_setxattr_cap'. The _owned hook would simply check the xattr name and return a positive value if the LSM "owned" the xattr, e.g. XATTR_NAME_SELINUX for SELinux, and zero otherwise. The _cap hook would only be used by the capabilities code (or something similar), and would match up with cap_inode_setxattr(). With these two new hooks I think we could do something like this: int security_inode_setxattr(...) { owned = false hook_loop(inode_setxattr_owned) { trc = hook->inode_setxattr_owned(name); if (trc > 0) { owned = true; break; } } if (owned) { hook_loop(inode_setxattr) { /* run the existing inode_setxattr hooks, e.g. SELinux and Smack */ } } else { hook_loop(inode_setxattr_cap) { /* run the capability setxattr hooks, e.g. commoncap.c */ } } } ... with security_inode_removexattr() following a similar pattern. I will admit that there is some duplication in having to check the xattr twice (once in _owned, again in inode_setxattr), and the multiple hook approach is less than ideal, but this seems much less fragile to me. Thoughts? -- paul-moore.com