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. -- 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.
