Re: [PATCH v2] fanotify: allow reporting errors on failure to open fd

[Amir Goldstein <amir73il@gmail.com>]

On Wed, Oct 16, 2024 at 5:48 PM Jan Kara <jack@suse.cz> wrote:
>
> Hello Amir!
>
> On Thu 03-10-24 16:29:22, Amir Goldstein wrote:
> > When working in "fd mode", fanotify_read() needs to open an fd
> > from a dentry to report event->fd to userspace.
> >
> > Opening an fd from dentry can fail for several reasons.
> > For example, when tasks are gone and we try to open their
> > /proc files or we try to open a WRONLY file like in sysfs
> > or when trying to open a file that was deleted on the
> > remote network server.
> >
> > Add a new flag FAN_REPORT_FD_ERROR for fanotify_init().
> > For a group with FAN_REPORT_FD_ERROR, we will send the
> > event with the error instead of the open fd, otherwise
> > userspace may not get the error at all.
> >
> > The FAN_REPORT_FD_ERROR flag is not allowed for groups in "fid mode"
> > which do not use open fd's as the object identifier.
> >
> > For ean overflow event, we report -EBADF to avoid confusing FAN_NOFD
> > with -EPERM.  Similarly for pidfd open errors we report either -ESRCH
> > or the open error instead of FAN_NOPIDFD and FAN_EPIDFD.
> >
> > In any case, userspace will not know which file failed to
> > open, so add a debug print for further investigation.
> >
> > Reported-by: Krishna Vivek Vitta <kvitta@microsoft.com>
> > Closes: https://lore.kernel.org/linux-fsdevel/SI2P153MB07182F3424619EDDD1F393EED46D2@SI2P153MB0718.APCP153.PROD.OUTLOOK.COM/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> I was mulling over this becase I wasn't quite happy with the result but I
> could not clearly formulate my problems with the patch. So I've just sat
> down and played with the code. Attached is what I've ended up with - please
> have a look if it looks OK to you as well, it passes the LTP test you've
> created. Functionally, I've just removed the check that FAN_REPORT_FD_ERROR
> cannot be used in "fid mode" because when we decided to use the flag for
> pidfd, it makes sense to combine it with "fid mode". I've also moved
> EOPENSTALE special handling to a more logical place now.
>

Yeh this looks nicer.
Thanks for the touch up.
Amir.