On Tue, Sep 27, 2022 at 07:11:17AM -0700, Casey Schaufler wrote: > On 9/27/2022 12:41 AM, Christoph Hellwig wrote: > > On Mon, Sep 26, 2022 at 05:22:45PM -0700, Casey Schaufler wrote: > >> I suggest that you might focus on the acl/evm interface rather than the entire > >> LSM interface. Unless there's a serious plan to make ima/evm into a proper LSM > >> I don't see how the breadth of this patch set is appropriate. > > Umm. The problem is the historically the Linux xattr interface was > > intended for unstructured data, while some of it is very much structured > > and requires interpretation by the VFS and associated entities. So > > splitting these out and add proper interface is absolutely the right > > thing to do and long overdue (also for other thing like capabilities). > > It might make things a little more verbose for LSM, but it fixes a very > > real problem. > > Here's the problem I see. All of the LSMs see xattrs, except for their own, > as opaque objects. Introducing LSM hooks to address the data interpretation > issues between VFS and EVM, which is not an LSM, adds to an already overlarge > and interface. And the "real" users of the interface don't need the new hook. > I'm not saying that the ACL doesn't have problems. I'm not saying that the > solution you've proposed isn't better than what's there now. I am saying that > using LSM as a conduit between VFS and EVM at the expense of the rest of the > modules is dubious. A lot of change to LSM for no value to LSM. > > I am not adamant about this. A whole lot worse has been done for worse reasons. > But as Paul says, we're overdue to make an effort to keep the LSM interface sane. So I assume the alternative you have in mind would be to use the existing setxattr hook? I worry about type confusion if an LSM does someday want to look inside the ACL data. Unless LSMs aren't supposed to look inside of xattr data, but in that case why pass the data pointer on to the LSMs? Note that the caller of this new hook does not have access to the uapi xattr data, and I think this is the right place for the new hook to be called as it's the interface that stacked filesystems like overlayfs will use to write ACLs to the lower filesystems. Seth
