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
> a event that contains ONLY flags for the event types that have been
^ an
> 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
> + * that have been specifically requested by the user. Flags that may have
> + * been included within the event mask, but have not been ecplicitly
^^ explicitly
> + * 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...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
