On Wed, Oct 12, 2022 at 6:44 PM Jan Kara <jack@xxxxxxx> wrote: > > Hi Amir! > > On Fri 07-10-22 16:58:21, Amir Goldstein wrote: > > > > The other use case of automatic inode marks I was thinking about, > > > > which are even more relevant for $SUBJECT is this: > > > > When instantiating a dentry with an inode that has xattr > > > > "security.fanotify.mask" (a.k.a. persistent inode mark), an inode > > > > mark could be auto created and attached to a group with a special sb > > > > mark (we can limit a single special mark per sb). > > > > This could be implemented similar to get_acl(), where i_fsnotify_mask > > > > is always initialized with a special value (i.e. FS_UNINITIALIZED) > > > > which is set to either 0 or non-zero if "security.fanotify.mask" exists. > > > > > > > > The details of how such an API would look like are very unclear to me, > > > > so I will try to focus on the recursive auto inode mark idea. > > > > > > Yeah, although initializing fanotify marks based on xattrs does not look > > > completely crazy I can see a lot of open questions there so I think > > > automatic inode mark idea has more chances for success at this point :). > > > > I realized that there is one sort of "persistent mark" who raises > > less questions - one that only has an ignore mask. > > > > ignore masks can have a "static" namespace that is not bound to any > > specific group, but rather a set of groups that join this namespace. > > > > I played with this idea and wrote some patches: > > https://github.com/amir73il/linux/commits/fan_xattr_ignore_mask > > I have glanced over the patches. In general the idea looks OK to me but I > have some concerns: > > 1) Technically, it may be challenging to call into filesystem xattr > handling code on first event generated by the inode - that may generate > some unexpected lock recursion for some filesystems and some events which > trigger the initialization... That may be a correct statement in general, but please note that - Only permission events trigger auto-init of xattr ignore mask - Permission events are called from security hooks - Security hooks may also call getxattr to get the security context Perhaps LSMs always initialize the security context in OPEN and never in ACCESS? One of the earlier versions of the patch initialized xattr ignore mask on LOOKUP permission event, if ANY object was interested in ANY permission event even if no object was interested in LOOKUP to mimic the LSM context initialization, but it was complicated and I wasn't sure if this was necessary. Maybe that is the way to go... > > 2) What if you set the xattr while the group is already listening to > events? Currently the change will get ignored, won't it? But I guess this > could be handled by clearing the "cached" flag when the xattr is set. > I have created an API to update the xattr via fanotify_mark(FAN_MARK_XATTR, ... which updates the cached ignore mask in the connector. I see no reason to support "direct" modifications of this xattr. If such changes are made directly it is fine to ignore them. > 3) What if multiple applications want to use the persistent mark > functionality? I think we need some way to associate a particular > fanotify group with a particular subset of fanotify xattrs so that > coexistence of multiple applications is possible... > Yeh, I thought about this as well. The API in the patches is quite naive because it implements a single global namespace for xattr ignore masks, but mostly I wanted to see if I can get the fast path and auto-init implementation done. I was generally thinking of ioctl() as the API to join an xattr marks namespace and negotiate the on-disk format of persistent marks supported by the application. I would not want to allow multiple fanotify xattrs per inode - that could have the consequence of the inode becoming a junkyard. I'd prefer to have a single xattr (say security.fanotify.mark) and that mark will have - on-disk format version - namespace id - ignore mask - etc If multiple applications want to use persistent marks they need to figure out how to work together without stepping on each other's toes. I don't think it is up to fanotify to coordinate that. fanotify_mark() can fail with EEXIST when a group that joined namespace A is trying to update a persistent mark when a persistent mark of namespace B already exists and probably some FAN_MARK_REPLACE flag could be used to force overwrite the existing persistent mark. Do you agree? Thanks, Amir.
