linux-kernel - Re: [PATCH] kernfs: attach uuid for every kernfs and report it in fsid

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: <CABWYdi2fTLnQeZnLG2uTCHyQkBnr1Z9OtfYhnzp_TtqiXyYd9A@mail.gmail.com>
Date:   Thu, 13 Jul 2023 14:34:18 -0700
From:   Ivan Babrou <ivan@...udflare.com>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-fsdevel@...r.kernel.org, kernel-team@...udflare.com,
        linux-kernel@...r.kernel.org, cgroups@...r.kernel.org,
        Tejun Heo <tj@...nel.org>, Hugh Dickins <hughd@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@....de>, Jan Kara <jack@...e.cz>,
        Zefan Li <lizefan.x@...edance.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH] kernfs: attach uuid for every kernfs and report it in fsid

On Tue, Jul 11, 2023 at 10:44 PM Amir Goldstein <amir73il@...il.com> wrote:
> > > I agree. I think it was a good decision.
> > > I have some followup questions though.
> > >
> > > I guess your use case cares about the creation of cgroups?
> > > as long as the only way to create a cgroup is via vfs
> > > vfs_mkdir() -> ... cgroup_mkdir()
> > > fsnotify_mkdir() will be called.
> > > Is that a correct statement?
> >
> > As far as I'm aware, this is the only way. We have the cgroups mailing
> > list CC'd to confirm.
> >
> > I checked systemd and docker as real world consumers and both use
> > mkdir and are visible in fanotify with this patch applied.
> >
> > > Because if not, then explicit fsnotify_mkdir() calls may be needed
> > > similar to tracefs/debugfs.
> > >
> > > I don't think that the statement holds for dieing cgroups,
> > > so explicit fsnotify_rmdir() are almost certainly needed to make
> > > inotify/fanotify monitoring on cgroups complete.
> > >
> > > I am on the fence w.r.t making the above a prerequisite to merging
> > > your patch.
> > >
> > > One the one hand, inotify monitoring of cgroups directory was already
> > > possible (I think?) with the mentioned shortcomings for a long time.
> > >
> > > On the other hand, we have an opportunity to add support to fanotify
> > > monitoring of cgroups directory only after the missing fsnotify hooks
> > > are added, making fanotify API a much more reliable option for
> > > monitoring cgroups.
> > >
> > > So I am leaning towards requiring the missing fsnotify hooks before
> > > attaching a unique fsid to cgroups/kernfs.
> >
> > Unless somebody responsible for cgroups says there's a different way
> > to create cgroups, I think this requirement doesn't apply.
> >
>
> I was more concerned about the reliability of FAN_DELETE for
> dieing cgroups without an explicit rmdir() from userspace.

I checked the code and cgroups are destroyed in cgroup_destroy_locked,
which is only called from two places:

* In cgroup_mkdir (only on failure)
* In cgroup_rmdir

See: https://elixir.bootlin.com/linux/v6.5-rc1/A/ident/cgroup_destroy_locked

We should be covered here.