On Thu, Feb 21, 2019 at 5:52 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: > On 2/21/2019 1:13 AM, Ondrej Mosnacek wrote: > > On Tue, Feb 19, 2019 at 5:43 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: > > ..... > >> The state you're maintaining is kernfs state, not LSM > >> infrastructure state. The state should be maintained in > >> kernfs, not in the LSM infrastructure. > > But I'm not maintaining any state. I'm merely trying to answer the > > query "Is there anything that will handle this hook? Do I need to > > prepare stuff for it?", which is obviously a query about the LSM > > state. Granted, ideally we wouldn't need to do any preparatory work at > > all, but that would require exposing more of the kernfs internals > > (which brings its own issues, but maybe I'll need to look into that > > approach more...). > > It sounds like you're bumping up against the limitations > of the finely honed optimized implementation of kernfs. :( > If it where still the pre-android era, when using an LSM > was rare, the check for an LSM might have made sense. Today, > with the vast majority of systems using LSMs*, optimizing for > the no LSM case is nonsensical. I imagine it might make sense on some very minimal embedded systems (microcontrollers?), where you almost certainly need kernfs (via sysfs or debugfs), but you don't necessarily need advanced security controls (you might just have a single monolithic userspace process running there and very limited memory/CPU resources). I'm hardly an expert on embedded platforms, but it sounds like a reasonable use case. (Although in such cases I'd expect CONFIG_SECURITY to be simply set to 'n', so no need for runtime checks anyway...). > > --- > * Android, Tizen, Fedora/RHEL, Ubuntu > > > ... > > Kernfs is an important component of the kernel. So is > > the security infrastructure. I would hope you don't want > > to turn this into a contest to see which maintainer has > > the biggest clout. > > Oh, no, you misunderstood my intention. I just got a feeling that this > > thread was turning into a discussion about perceived code ugliness > > (and about which subsystem that ugliness ends up in), which is > > naturally a very subjective topic, so I wanted to know what is the > > opinion of the people that have the final decision about whether the > > code should get in or not. Anyway, I'll try to find a more elegant > > variant of the solution once again, hopefully I manage to get to > > something less controversial. > > Thank you. I believe (which, of course, doesn't make it true) > that when a component goes outside the general system architecture > the way that kernfs does *even for performance reasons* that it is > responsible for the edge cases it encounters. I know that I've had > to do a good bit of that in Smack. OK, so I tried taking the other approach (pass kernfs nodes and expose necessary kernfs internals) and I'm quite happy with the result. It turns out the only thing I actually need to expose (at least for SELinux) is getting and setting the security xattrs, which is just two simple functions. The rest of the attributes (uid, gid, and access times) don't seem important and they can always be exposed by adding more helper functions as needed. With this design the last patch now becomes embarrassingly simple - there is just a single call that will either call an LSM hook or do nothing at all. I still need to do some testing on the new patches before posting. In the meantime they are available here for the curious: https://gitlab.com/omos/linux-public/compare/selinux-next...selinux-fix-cgroupfs-v12 -- Ondrej Mosnacek <omosnace at redhat dot com> Associate Software Engineer, Security Technologies Red Hat, Inc.
