linux-ext4 - Re: [PATCH v8 10/19] fanotify: introduce FAN_PRE

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: <CAOQ4uxhsEA2zj-a6H+==S+6G8nv+BQEJDoGjJeimX0yRhHso2w@mail.gmail.com>
Date: Thu, 21 Nov 2024 19:31:20 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: Josef Bacik <josef@...icpanda.com>, kernel-team@...com, linux-fsdevel@...r.kernel.org, 
	brauner@...nel.org, torvalds@...ux-foundation.org, viro@...iv.linux.org.uk, 
	linux-xfs@...r.kernel.org, linux-btrfs@...r.kernel.org, linux-mm@...ck.org, 
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH v8 10/19] fanotify: introduce FAN_PRE_ACCESS permission event

On Thu, Nov 21, 2024 at 5:36 PM Jan Kara <jack@...e.cz> wrote:
>
> On Thu 21-11-24 15:18:36, Amir Goldstein wrote:
> > On Thu, Nov 21, 2024 at 11:44 AM Jan Kara <jack@...e.cz> wrote:
> > >
> > > On Fri 15-11-24 10:30:23, Josef Bacik wrote:
> > > > From: Amir Goldstein <amir73il@...il.com>
> > > >
> > > > Similar to FAN_ACCESS_PERM permission event, but it is only allowed with
> > > > class FAN_CLASS_PRE_CONTENT and only allowed on regular files and dirs.
> > > >
> > > > Unlike FAN_ACCESS_PERM, it is safe to write to the file being accessed
> > > > in the context of the event handler.
> > > >
> > > > This pre-content event is meant to be used by hierarchical storage
> > > > managers that want to fill the content of files on first read access.
> > > >
> > > > Signed-off-by: Amir Goldstein <amir73il@...il.com>
> > >
> > > Here I was wondering about one thing:
> > >
> > > > +     /*
> > > > +      * Filesystems need to opt-into pre-content evnets (a.k.a HSM)
> > > > +      * and they are only supported on regular files and directories.
> > > > +      */
> > > > +     if (mask & FANOTIFY_PRE_CONTENT_EVENTS) {
> > > > +             if (!(path->mnt->mnt_sb->s_iflags & SB_I_ALLOW_HSM))
> > > > +                     return -EINVAL;
> > > > +             if (!is_dir && !d_is_reg(path->dentry))
> > > > +                     return -EINVAL;
> > > > +     }
> > >
> > > AFAICS, currently no pre-content events are generated for directories. So
> > > perhaps we should refuse directories here as well for now? I'd like to
> >
> > readdir() does emit PRE_ACCESS (without a range)
>
> Ah, right.
>
> > and also always emitted ACCESS_PERM.
>
> I know that and it's one of those mostly useless events AFAICT.
>
> > my POC is using that PRE_ACCESS to populate
> > directories on-demand, although the functionality is incomplete without the
> > "populate on lookup" event.
>
> Exactly. Without "populate on lookup" doing "populate on readdir" is ok for
> a demo but not really usable in practice because you can get spurious
> ENOENT from a lookup.
>
> > > avoid the mistake of original fanotify which had some events available on
> > > directories but they did nothing and then you have to ponder hard whether
> > > you're going to break userspace if you actually start emitting them...
> >
> > But in any case, the FAN_ONDIR built-in filter is applicable to PRE_ACCESS.
>
> Well, I'm not so concerned about filtering out uninteresting events. I'm
> more concerned about emitting the event now and figuring out later that we
> need to emit it in different places or with some other info when actual
> production users appear.
>
> But I've realized we must allow pre-content marks to be placed on dirs so
> that such marks can be placed on parents watching children. What we'd need
> to forbid is a combination of FAN_ONDIR and FAN_PRE_ACCESS, wouldn't we?

Yes, I think that can work well for now.

Thanks,
Amir.