Re: [PATCH -next 0/2] lsm: Change inode_setattr() to take struct

[Christian Brauner <brauner@kernel.org>]

On Fri, May 05, 2023 at 04:11:58PM +0800, Xiu Jianfeng wrote:
> Hi,
> 
> I am working on adding xattr/attr support for landlock [1], so we can
> control fs accesses such as chmod, chown, uptimes, setxattr, etc.. inside
> landlock sandbox. the LSM hooks as following are invoved:
> 1.inode_setattr
> 2.inode_setxattr
> 3.inode_removexattr
> 4.inode_set_acl
> 5.inode_remove_acl
> which are controlled by LANDLOCK_ACCESS_FS_WRITE_METADATA.
> 
> and
> 1.inode_getattr
> 2.inode_get_acl
> 3.inode_getxattr
> 4.inode_listxattr
> which are controlled by LANDLOCK_ACCESS_FS_READ_METADATA

It would be helpful to get the complete, full picture.

Piecemeal extending vfs helpers with struct path arguments is costly,
will cause a lot of churn and will require a lot of review time from us.

Please give us the list of all security hooks to which you want to pass
a struct path (if there are more to come apart from the ones listed
here). Then please follow all callchains and identify the vfs helpers
that would need to be updated. Then please figure out where those
vfs helpers are called from and follow all callchains finding all
inode_operations that would have to be updated and passed a struct path
argument. So ultimately we'll end up with a list of vfs helpers and
inode_operations that would have to be changed.

I'm very reluctant to see anything merged without knowing _exactly_ what
you're getting us into.