On Mon 11-12-23 14:00:58, Amir Goldstein wrote:
> On Mon, Dec 11, 2023 at 1:49 PM Jan Kara <jack@xxxxxxx> wrote:
> >
> > On Sun 10-12-23 15:24:00, Amir Goldstein wrote:
> > > > > > 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.
> >
> > OK, understood. But isn't anything with len == 0 in fact "no valid range
> > provided" case? So we could use that to identify a case where we simply
> > don't report any range with the event without a need to pass the pointer?
> >
>
> IDK. read(2) and pread(2) with count=0 is a valid use case.
> and we have reported FAN_ACCESS_PERM for those 0 length calls so far.
> reporting those call with no range would be bad for HSM, because all
> HSM can do with these events is to fill the entire file content.
Yes, reading or writing 0 bytes is valid but I was wondering whether we
are issuing event for that. Now that I've checked we indeed do so let's not
complicate this further...
> Filling the entire file content is a valid action if the backing file does not
> support seek or if it is a directory, but it is not a valid action for
> an application
> induced pread() with zero length.
>
> Did I misunderstand what you meant?
Yeah, ok, let's leave the calling convention as you have it now. We can
always change it when we come up with something more elegant.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
