Re: [PATCH AUTOSEL 4.20 016/117] fanotify: return only user requested event types in event mask

From: Jan Kara <jack@suse.cz>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Sasha Levin <sashal@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	stable <stable@vger.kernel.org>,
	Matthew Bobrowski <mbobrowski@mbobrowski.org>,
	Jan Kara <jack@suse.cz>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH AUTOSEL 4.20 016/117] fanotify: return only user requested event types in event mask
Date: Wed, 9 Jan 2019 12:32:42 +0100	[thread overview]
Message-ID: <20190109113242.GE15397@quack2.suse.cz> (raw)
Message-ID: <20190109113242.KNgX_L9xRV83BTwmQN56uK29orUv3JUkJrehJt5W0_g@z> (raw)
In-Reply-To: <CAOQ4uxgwZjyoEOeRRj0eRsPR2+QmyUht0sErtZV=meKcPTDVcA@mail.gmail.com>

On Wed 09-01-19 08:50:33, Amir Goldstein wrote:
> On Tue, Jan 8, 2019 at 10:11 PM Sasha Levin <sashal@kernel.org> wrote:
> >
> > From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> >
> > [ Upstream commit 2d10b23082a7eb8be508b3789f2e7250a88a5ddb ]
> >
> > Modify fanotify_should_send_event() so that it now returns a mask for
> > an event that contains ONLY flags for the event types that have been
> > 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@mbobrowski.org>
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > ---
> 
> I have no objection to applying this patch to 4.20, but FYI, it does not
> fix anything.  Before introducing FAN_OPEN_EXEC in 5.0-rc1, this patch
> has no visible effect.

Yes, the patch is just a code refactoring useful for the FAN_OPEN_EXEC
feature.

> I don't mind if you apply it. It will make stable code closer to
> mainline, which is always a good thing IMO. And FWIW, I think that patch
> is quite trivial and low risk.

I don't think applying code refactoring to stable is a good idea. Every
change has a risk of regression and this particular one brings users no
benefit. So I'd prefer to drop this patch from stable queue.

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