On Tue, Oct 16, 2018 at 01:52:15PM +0200, Jan Kara wrote:
>
> Hi,
>
> the patch looks good. Just couple nits:
>
> On Thu 11-10-18 21:42:41, Matthew Bobrowski wrote:
> > Modified fanotify_should_send_event() so that it now returns a mask for
> ^^ Modify
Updated.
> > a event that contains ONLY flags for the event types that have been
> ^ an
Updated.
> > specifically requested by the user. Flags that may have been included
> > within the event mask, but have not been explicitly requested by the
> > user will not be present in the returned value.
> >
> > As an example, given the situation where a user requests events of type
> > FAN_OPEN. Traditionally, the event mask returned within an event that
> > occurred on a filesystem object that has been marked for monitoring and is
> > opened, will only ever have the FAN_OPEN bit set. With the introduction of
> > the new flags like FAN_OPEN_EXEC, and perhaps any other future event
> > flags, there is a possibility of the returned event mask containing more
> > than a single bit set, despite having only requested the single event type.
> > Prior to these modifications performed to fanotify_should_send_event(), a
> > user would have received a bundled event mask containing flags FAN_OPEN
> > and FAN_OPEN_EXEC in the instance that a file was opened for execution via
> > execve(), for example. This means that a user would receive event types
> > in the returned event mask that have not been requested. This runs the
> > possibility of breaking existing systems and causing other unforeseen
> > issues.
> >
> > To mitigate this possibility, fanotify_should_send_event() has been
> > modified to return the event mask containing ONLY event types explicitly
> > requested by the user. This means that we will NOT report events that the
> > user did no set a mask for, and we will NOT report events that the user
> > has set an ignore mask for.
> >
> > The function name fanotify_should_send_event() has also been updated so
> > that it's more relevant to what it has been designed to do.
> >
> > Signed-off-by: Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx>
> > ---
> > fs/notify/fanotify/fanotify.c | 46 ++++++++++++++++++++---------------
> > 1 file changed, 26 insertions(+), 20 deletions(-)
> >
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index b3e92302ed84..9da334d343b8 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -89,7 +89,13 @@ static int fanotify_get_response(struct fsnotify_group *group,
> > return ret;
> > }
> >
> > -static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
> > +/*
> > + * This function returns a mask for a event that only contains the flags
> ^^ an
Updated.
> > + * that have been specifically requested by the user. Flags that may have
> > + * been included within the event mask, but have not been ecplicitly
> ^^ explicitly
Updated.
> > + * requested by the user, will not be present in the returned mask.
> > + */
> > +static u32 fanotify_group_event_mask(struct fsnotify_iter_info *iter_info,
> > u32 event_mask, const void *data,
> > int data_type)
>
> <snip>
>
> > struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
> > @@ -194,6 +197,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> > struct fsnotify_iter_info *iter_info)
> > {
> > int ret = 0;
> > + u32 event_mask = 0;
> > struct fanotify_event_info *event;
> > struct fsnotify_event *fsn_event;
> >
> > @@ -211,13 +215,15 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> >
> > BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
> >
> > - if (!fanotify_should_send_event(iter_info, mask, data, data_type))
> > + event_mask = fanotify_group_event_mask(iter_info, mask, data,
> > + data_type);
> ^^^ Why don't you store the result in 'mask'? You don't need
> the original mask later anyway, it reduces churn and also possibility of
> getting things wrong in the future...
Good point. There's no real reason why we can't overwrite the original
'mask' value with the returned value from fanotify_group_event_mask().
That being said, I've gone ahead and applied updates accordingly.
I will post through the updates within an updated patch series.
