Re: [LTP] [PATCH 3/3] fanotify24: add mmap() and user page fault to test

[Petr Vorel <pvorel@suse.cz>]

> On Thu, Mar 20, 2025 at 6:00 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Amir,

> > > Commit 066e053fe208a ("fsnotify: add pre-content hooks on mmap()")
> > > in 6.14-rc7 added a pre-content hooh in mmap() before the final release
> > > of the fsnotify pre-content event feature.

> > > To test pre-content hook on mmap(), increase the size of the test file
> > > to 101 pages use mmap() to setup a buffer from the end of the test file.

> > > Change some of the test cases to allow the write() and use this buffer
> > > as the input buffer to write(), which tests deadlock avoidance by
> > > suppresing pre-content hook on user page faults.

> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >  .../kernel/syscalls/fanotify/fanotify24.c     | 41 ++++++++++++++++---
> > >  1 file changed, 35 insertions(+), 6 deletions(-)

> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify24.c b/testcases/kernel/syscalls/fanotify/fanotify24.c
> > > index 2d2479b39..3a76f2c1b 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify24.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify24.c
> > > @@ -46,6 +46,7 @@
> > >  static char fname[BUF_SIZE];
> > >  static char buf[BUF_SIZE];
> > >  static volatile int fd_notify;
> > > +static size_t page_sz;

> > >  static pid_t child_pid;

> > > @@ -68,7 +69,8 @@ static struct tcase {
> > >               FAN_OPEN_PERM | FAN_PRE_ACCESS,
> > >               {
> > >                       {FAN_OPEN_PERM, FAN_ALLOW},
> > > -                     {FAN_PRE_ACCESS, FAN_DENY},
> > > +                     {FAN_PRE_ACCESS, FAN_ALLOW},
> > > +                     {FAN_PRE_ACCESS, FAN_ALLOW},

> > If I understand correctly, these doubled FAN_PRE_ACCESS, FAN_ALLOW are for
> > reusing the write buffer, right? Obviously it's needed but on the first look it
> > looks like error.

> Sorry, this wasn't properly explained.

> This change adds a test for pre-mmap events.

> First of all, we added an mmap() call before the write(),
> so we need to add another expected FAN_PRE_ACCESS
> for the mmap() and we allow the mmap for all test cases.

> Secondly, we Change some of the test cases to allow the write()
> and use the mmaped buffer as the input buffer.

> The reason that we allow the write() is to verify that we do not
> get another FAN_PRE_ACCESS event from reading from the
> mmaped input buffer.

Thanks for the explanation. It would deserve to update the commit message,
but it's probably obvious to Jan and I add link to lore before merging,
therefore maybe it's enough as is.

Anyway, whole patchset LGTM. Waiting till tomorrow before merging if Jan has
some input.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> Thanks,
> Amir.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp