Stephen Smalley wrote: > On Tue, 2009-08-18 at 21:37 -0700, Casey Schaufler wrote: > >> Stephen Smalley wrote: >> >>> On Tue, 2009-08-18 at 07:12 -0700, Casey Schaufler wrote: >>> >>> >>>> Stephen Smalley wrote: >>>> >>>> >>>>> On Mon, 2009-08-17 at 20:55 -0700, Casey Schaufler wrote: >>>>> >>>>> >>>>> >>>>>> From: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> >>>>>> >>>>>> Another approach to limited xattr support in sysfs. >>>>>> >>>>>> I tried to listen to the objections to a linked list representation >>>>>> and I think that I understand that there isn't really any interest >>>>>> in supporting xattrs for real, only for those maintained by LSMs. >>>>>> I also looked carefully into the claims that memory usage is >>>>>> critical and that the code I had before was duplicating effort. >>>>>> >>>>>> This version lets the surrounding code do as much of the work as >>>>>> possible. Unlike the initial proposal for sysfs xattrs, it does not >>>>>> introduce any new LSM hooks, it uses hooks that already exist. It >>>>>> does not support any attributes on its own, it only provides for >>>>>> the attribute advertised by security_inode_listsecurity(). It could >>>>>> easily be used by other filesystems to provide the same LSM xattr >>>>>> support. It could also be extended to do the list based support for >>>>>> arbitrary xattrs without too much effort. >>>>>> >>>>>> Probably the oddest bit is that the inode_getsecurity hooks need to >>>>>> check to see if they are getting called before the inode is instantiated >>>>>> and return -ENODATA in that event. It would be possible to do a >>>>>> filesystem specific check instead, but this way provides for generally >>>>>> correct behavior at small cost. >>>>>> >>>>>> This has been tested with Smack, but not SELinux. I think that >>>>>> SELinux will work correctly, but it could be that a labeling >>>>>> behavior that is different than the "usual" instantiation labeling >>>>>> is actually desired. That would be an easy change. >>>>>> >>>>>> As always, let me know if I missed something obvious or if there's a >>>>>> fatal flaw in the scheme. >>>>>> >>>>>> >>>>>> >>>>> The point of the David's patch was to provide a way to save the security >>>>> xattr in the backing data structure for sysfs entries when an attribute >>>>> value is set from userspace so that the value can be preserved if the >>>>> inode is evicted from memory and later re-instantiated. AFAICS, your >>>>> patch completely misses the problem. How about we just go back to >>>>> David's patch? >>>>> >>>>> >>>>> >>>> Oh no, that would use too much memory! >>>> >>>> Either you care about the value the user set, in which case you >>>> want to save the value the user set, or you don't. If you do, you >>>> have to save that value, not an LSM's interpretation of that value. >>>> No secids. No new hooks. >>>> >>>> >>> As the security module is the only component of the kernel that >>> uses/interprets that value, it isn't unreasonable for the security >>> module's interpretation of that value to be considered canonical. In >>> fact, that is already the case - the hooks within vfs_getxattr() enable >>> the security module to override/replace the actual security xattr value >>> returned to userspace. >>> >>> In the case of Smack, Smack could just provide a pointer to its own >>> internal copy of the string, and that could be stored in the wrapped >>> iattr. In the case of SELinux, we could provide a secid that could be >>> stored in the wrapped iattr. The hook interface could just handle it as >>> a blob if you prefer. But either way we don't need extra storage aside >>> from a pointer-size field in the wrapped iattr. >>> >>> >> So how often is the SELinux label going to get explicitly set in /sys ? >> I'm grappling with the value of going hog-wild in optimizing this. If >> it is something that's quite rare I can see the concern with expanding >> the d_entry. If it is common, the storage associated with storing the >> xattr could be an issue. If it is uncommon but not rare there's another >> story again. >> >> I'm looking at addressing the issues. Thank you. >> > > I'd expect most sysfs nodes to be left in the default label, although we > don't really know as this would be the first time that people have the > option of finer-grained control to sysfs. > > So I still don't like the way it exposes LSM internal data to the file system code, but given how long it's taking for me to create a better solution I don't think that I can in all fairness say NAK to David Quigley's sysfs patch any longer. I withdraw my objection, while maintaining my reservations. -- This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with the words "unsubscribe selinux" without quotes as the message.
