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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 23 Jan 2019 08:56:29 -0500
From:   Sasha Levin <sashal@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     Amir Goldstein <amir73il@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Matthew Bobrowski <mbobrowski@...browski.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH AUTOSEL 4.20 016/117] fanotify: return only user
 requested event types in event mask

On Wed, Jan 09, 2019 at 12:32:42PM +0100, Jan Kara wrote:
>On Wed 09-01-19 08:50:33, Amir Goldstein wrote:
>> On Tue, Jan 8, 2019 at 10:11 PM Sasha Levin <sashal@...nel.org> wrote:
>> >
>> > From: Matthew Bobrowski <mbobrowski@...browski.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@...browski.org>
>> > Reviewed-by: Amir Goldstein <amir73il@...il.com>
>> > Signed-off-by: Jan Kara <jack@...e.cz>
>> > Signed-off-by: Sasha Levin <sashal@...nel.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.

No objections there, dropping it. Thank you.

--
Thanks,
Sasha