From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-f196.google.com ([209.85.215.196]:41461 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727080AbeJQOYV (ORCPT ); Wed, 17 Oct 2018 10:24:21 -0400 Received: by mail-pg1-f196.google.com with SMTP id 23-v6so12015399pgc.8 for ; Tue, 16 Oct 2018 23:30:14 -0700 (PDT) Date: Wed, 17 Oct 2018 17:30:07 +1100 From: Matthew Bobrowski To: Jan Kara Cc: amir73il@gmail.com, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, sgrubb@redhat.com Subject: Re: [PATCH v4 2/3] fanotify: return only user requested event types in event mask Message-ID: <20181017063005.GA1580@development.internal.lab> References: <20181016115215.GI18918@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181016115215.GI18918@quack2.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 > > --- > > 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) > > > > > 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.