On Sun 13-10-24 18:34:18, Amir Goldstein wrote:
> On Fri, May 24, 2024 at 2:35 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > On Fri, May 24, 2024 at 1:19 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > AFAICT, the only thing that currently makes this impossible in this patch
> > is the O_DIRECTORY limitation.
> > Because there could be non-directory non-hashed aliases of deleted
> > files in dcache.
> >
>
> This limitation, that you added to my request, has implications on
> fanotify (TLDR):
>
> file handles reported in fanotify events extra info of type
> FAN_EVENT_INFO_TYPE_DFID{,_NAME} due to fanotify_inide()
> flag FAN_REPORT_DIR_FID are eligible for open_by_handle_at(2)
> inside non-root userns and FAN_REPORT_DFID_NAME is indeed
> the most expected use case - namely
> directory file handle can always be used to find the parent's path
> and the name of the child entry is provided in the event.
>
> HOWEVER,
> with fanotify_init() flags FAN_REPORT_{TARGET_,}FID
> the child file handle is reported additionally, using extra info type
> FAN_EVENT_INFO_TYPE_FID and this file handle may not be
> eligible for open_by_handle_at(2) inside non-root userns,
> because it could be resolved to a disconnected dentry, for which
> permission cannot be checked.
>
> I don't think this is a problem - the child fid is supposed to
> be used as an extra check to verify with name_to_handle_at()
> that the parent dir and child name refer to the same object as
> the one reported with the child fid.
> But it can be a bit confusing to users that some file handles
> (FAN_EVENT_INFO_TYPE_DFID) can be decoded and some
> (FAN_EVENT_INFO_TYPE_FID) cannot, so this hairy detail
> will need to be documented.
Well, I think the explanation that open_by_handle_at() works only for
directories inside containers is clear enough explanation. It has nothing
to do with whether the FID comes from FAN_EVENT_INFO_TYPE_DFID or from
FAN_EVENT_INFO_TYPE_FID.
> In retrospect, for unpriv fanotify, we should not have allowed
> the basic mode FAN_REPORT_FID, which does not make a
> distinction between directory and non-dir file handles.
I don't think we could have forseen these changes. Also I don't think the
interface is wrong per se. Just the handle cannot be used with
open_by_handle_at() if it points to a regular file so it may be annoying to
use but we didn't expect the handle will ever be usable with
open_by_handle_at() for unpriviledged users so making it usable for
directories can be seen as an improvement :). Realistically I think
unpriviledged users will use FAN_EVENT_INFO_TYPE_DFID_NAME to avoid these
headaches.
> Jan, do you think we should deny the FAN_REPORT_FID mode
> with non-root userns sb/mount watches to reduce some unneeded
> rows in the test matrix?
I think this would be a bit arbitrary restriction. I don't see how
supporting FAN_REPORT_FID would be making our life any harder.
> Any other thoughts regarding non-root userns sb/mount watches?
No, not really.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
