Re: [PATCH 4/4] fsnotify: pass access range in file permission hooks

From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>,
	Jeff Layton <jlayton@kernel.org>,
	 Josef Bacik <josef@toxicpanda.com>,
	Christoph Hellwig <hch@lst.de>,
	David Howells <dhowells@redhat.com>,
	 Jens Axboe <axboe@kernel.dk>, Miklos Szeredi <miklos@szeredi.hu>,
	Al Viro <viro@zeniv.linux.org.uk>,
	 linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 4/4] fsnotify: pass access range in file permission hooks
Date: Sun, 10 Dec 2023 15:24:00 +0200	[thread overview]
Message-ID: <CAOQ4uxiLtwp1QLQN1VBa10kLf4z+dx=UiDtB_WSqNXcoLYbvfw@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxi+6VMAdyREODOpMLiZ26Q_1R981H-eOwOA8gJsrsSqrA@mail.gmail.com>

> > > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> > > index 0a9d6a8a747a..45e6ecbca057 100644
> > > --- a/include/linux/fsnotify.h
> > > +++ b/include/linux/fsnotify.h
> > > @@ -103,7 +103,8 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
> > >  /*
> > >   * fsnotify_file_perm - permission hook before file access
> > >   */
> > > -static inline int fsnotify_file_perm(struct file *file, int perm_mask)
> > > +static inline int fsnotify_file_perm(struct file *file, int perm_mask,
> > > +                                  const loff_t *ppos, size_t count)
> > >  {
> > >       __u32 fsnotify_mask = FS_ACCESS_PERM;
> >
> > Also why do you actually pass in loff_t * instead of plain loff_t? You
> > don't plan to change it, do you?
>
> No I don't.

Please note that the pointer is to const loff_t.

>
> I used NULL to communicate "no range info" to fanotify.
> It is currently only used from iterate_dir(), but filesystems may need to
> use that to report other cases of pre-content access with no clear range info.

Correction. iterate_dir() is not the only case.
The callers that use file_ppos(), namely ksys_{read,write}, do_{readv,writev}()
will pass a NULL ppos for an FMODE_STREAM file.
The only sane behavior I could come up with for those cases
is to not report range_info with the FAN_PRE_ACCESS event.

>
> I could leave fsnotify_file_perm(file, mask) for reporting events without
> range info and add fsnotify_file_area(file, mask, pos, count) for reporting
> access permission with range info.
>

I renamed the hook in v2 to fsnotify_file_area_perm() and added a wrapper:

static inline int fsnotify_file_perm(struct file *file, int perm_mask)
{
        return fsnotify_file_area_perm(file, perm_mask, NULL, 0);
}

Thanks,
Amir.