On Wed, Sep 25, 2024 at 1:38 PM Jan Kara <jack@xxxxxxx> wrote:
>
> [Adding 9p guys to CC]
>
> On Wed 25-09-24 12:51:38, Amir Goldstein wrote:
> > On Wed, Sep 25, 2024 at 10:18 AM Jan Kara <jack@xxxxxxx> wrote:
> > > On Wed 25-09-24 10:11:46, Jan Kara wrote:
> > > > On Tue 24-09-24 12:07:51, Krishna Vivek Vitta wrote:
> > > > > Please ignore the last line.
> > > > > Git clone operation is failing with fanotify example code as well.
> > > > >
> > > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue# ./fanotify_ex /mnt/c
> > > > > Press enter key to terminate.
> > > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue# ./fanotify_ex /mnt/c
> > > > > Press enter key to terminate.
> > > > > Listening for events.
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/info/exclude
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/info/exclude
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-applypatch.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-applypatch.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/applypatch-msg.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/applypatch-msg.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/commit-msg.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/commit-msg.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-push.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-push.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-merge-commit.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-merge-commit.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-commit.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-commit.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/post-update.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/post-update.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/push-to-checkout.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/push-to-checkout.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/fsmonitor-watchman.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/fsmonitor-watchman.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/update.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/update.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-rebase.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-rebase.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-receive.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/pre-receive.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/hooks/prepare-commit-msg.sample
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/hooks/prepare-commit-msg.sample
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/description
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/description
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/HEAD.lock
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/HEAD.lock
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config.lock
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_CLOSE_WRITE: File /mnt/c/Users/kvitta/gtest/.git/config
> > > > > FAN_OPEN_PERM: File /mnt/c/Users/kvitta/gtest/.git/tNbqjiA
> > > > > read: No such file or directory
> > > > > root@MININT-S244RA7:/mnt/c/Users/kvitta/Desktop/MDE binaries/GitCloneIssue#
> > > >
> > > > OK, so it appears that dentry_open() is failing with ENOENT when we try to
> > > > open the file descriptor to return with the event. This is indeed
> > > > unexpected from the filesystem.
> >
> > How did you conclude that is what is happening?
> > Were you able to reproduce, because I did not.
>
> No, I didn't reproduce. But checking the source of the reproducer the
> message "read: No such file or directory" means fanotify_read() has
> returned -ENOENT and within fanotify there's no way how that could happen.
> So the only possible explanation is that dentry_open() returns it.
>
Ah, I missed that bit in the report.
> > > > On the other hand we already do silently
> > > > fixup similar EOPENSTALE error that can come from NFS so perhaps we should
> > > > be fixing ENOENT similarly? What do you thing Amir?
> > >
> >
> > But we never return this error to the caller for a non-permission event,
> > so what am I missing?
>
> Umm. If dentry_open() fails, we return the error from copy_event_to_user()
> without copying anything. Then fanotify_read() does:
>
> ret = copy_event_to_user(group, event, buf, count);
> ...
> if (!fanotify_is_perm_event(event->mask)) {
> fsnotify_destroy_event(group, &event->fse);
> // unused event destroyed
> } else {
> ...
> }
> if (ret < 0)
> break; // read loop aborted
> ...
> }
> ...
> if (start != buf && ret != -EFAULT)
> ret = buf - start;
> return ret;
>
> So the error *is* IMO returned to userspace if this was the first event.
>
Yes it is "returned to userspace" but it is returned to the fanotify listener.
The original complaint is about the error returned to open(2) after rename(2)
in the rename_try.c reproducer (when fanotify is watching close events).
I admit that fanotify_read() randomly returning an error is sub-optimal,
but I do prefer it over dropping the error silently.
BTW, there are several other dentry_open() errors that are easy to
reproduce in this context:
ENXIO after an event on a pipe with no reader
ENODEV after an event on a device node with no backing instance
These were part of the reason that I excluded special files from
new pre-content events...
Thanks,
Amir.
