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

[Jan Kara <jack@suse.cz>]

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?

> > 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);
> }

Otherwise this works for me as well.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR