linux-ext4 - Re: [PATCH v6 06/17] fsnotify: generate pre-content permission event on open

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]

Message-ID: <20241112135457.zxzhtoe537gapkmu@quack3>
Date: Tue, 12 Nov 2024 14:54:57 +0100
From: Jan Kara <jack@...e.cz>
To: Amir Goldstein <amir73il@...il.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	Josef Bacik <josef@...icpanda.com>, kernel-team@...com,
	linux-fsdevel@...r.kernel.org, jack@...e.cz, brauner@...nel.org,
	linux-xfs@...r.kernel.org, linux-btrfs@...r.kernel.org,
	linux-mm@...ck.org, linux-ext4@...r.kernel.org
Subject: Re: [PATCH v6 06/17] fsnotify: generate pre-content permission event
 on open

On Tue 12-11-24 09:11:32, Amir Goldstein wrote:
> On Tue, Nov 12, 2024 at 1:37 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> > On Mon, 11 Nov 2024 at 16:00, Amir Goldstein <amir73il@...il.com> wrote:
> > >
> > > I think that's a good idea for pre-content events, because it's fine
> > > to say that if the sb/mount was not watched by a pre-content event listener
> > > at the time of file open, then we do not care.
> >
> > Right.
> >
> > > The problem is that legacy inotify/fanotify watches can be added after
> > > file is open, so that is allegedly why this optimization was not done for
> > > fsnotify hooks in the past.
> >
> > So honestly, even if the legacy fsnotify hooks can't look at the file
> > flag, they could damn well look at an inode flag.
> 
> Legacy fanotify has a mount watch (FAN_MARK_MOUNT),
> which is the common way for Anti-malware to set watches on
> filesystems, so I am not sure what you are saying.
> 
> > And I'm not even convinced that we couldn't fix them to just look at a
> > file flag, and say "tough luck, somebody opened that file before you
> > started watching, you don't get to see what they did".
> 
> That would specifically break tail -f (for inotify) and probably many other
> tools, but as long as we also look at the inode flags (i_fsnotify_mask)
> and the dentry flags (DCACHE_FSNOTIFY_PARENT_WATCHED),
> then I think we may be able to get away with changing the semantics
> for open files on a fanotify mount watch.

Yes, I agree we cannot afford to generate FS_MODIFY event only if the mark
was placed after file open. There's too much stuff in userspace depending
on this since this behavior dates back to inotify interface sometime in
2010 or so.

> Specifically, I would really like to eliminate completely the cost of
> FAN_ACCESS_PERM event, which could be gated on file flag, because
> this is only for security/Anti-malware and I don't think this event is
> practically
> useful and it sure does not need to guarantee permission events to mount
> watchers on already open files.

For traditional fanotify permission events I agree generating them only if
the mark was placed before open is likely fine but we'll have to try and
see whether something breaks. For the new pre-content events I like the
per-file flag as Linus suggested. That should indeed save us some cache
misses in some fast paths.

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