linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH][RFC] ->mnt_devname is never NULL
Date: Wed, 23 Apr 2025 23:20:45 +0100	[thread overview]
Message-ID: <20250423222045.GF2023217@ZenIV> (raw)
In-Reply-To: <20250421162947.GW2023217@ZenIV>

On Mon, Apr 21, 2025 at 05:29:47PM +0100, Al Viro wrote:
> On Mon, Apr 21, 2025 at 09:56:20AM +0200, Christian Brauner wrote:
> > On Mon, Apr 21, 2025 at 04:35:09AM +0100, Al Viro wrote:
> > > Not since 8f2918898eb5 "new helpers: vfs_create_mount(), fc_mount()"
> > > back in 2018.  Get rid of the dead checks...
> > >     
> > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > > ---
> > 
> > Good idea. Fwiw, I've put this into vfs-6.16.mount with some other minor
> > stuff. If you're keeping it yourself let me know.
> 
> Not sure...  I'm going through documenting the struct mount lifecycle/locking/etc.
> and it already looks like there will be more patches, but then some are going
> to be #fixes fodder.

BTW, could you explain what this is about?
        /*
         * If this is an attached mount make sure it's located in the callers
         * mount namespace. If it's not don't let the caller interact with it.
         *
         * If this mount doesn't have a parent it's most often simply a
         * detached mount with an anonymous mount namespace. IOW, something
         * that's simply not attached yet. But there are apparently also users
         * that do change mount properties on the rootfs itself. That obviously
         * neither has a parent nor is it a detached mount so we cannot
         * unconditionally check for detached mounts.
         */
        if ((mnt_has_parent(mnt) || !is_anon_ns(mnt->mnt_ns)) && !check_mnt(mnt))
                goto out;

Why do you care about mnt_has_parent() here?  mnt is the root of subtree you
are operating on, so that condition means
	* any subtree (including the entire tree) of caller's mount tree is OK
(fair enough)
	* full mount tree of anon namespace is OK
	* nothing else is acceptable

What about partial subtrees of anon namespaces?  Restriction looks odd...

  parent reply	other threads:[~2025-04-23 22:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-21  3:35 [PATCH][RFC] ->mnt_devname is never NULL Al Viro
2025-04-21  7:56 ` Christian Brauner
2025-04-21 16:29   ` Al Viro
2025-04-21 17:03     ` Al Viro
2025-04-22  3:14       ` [PATCH][RFC] do_lock_mount() races in 'beneath' case Al Viro
2025-04-22  7:47         ` Christian Brauner
2025-04-22  7:43       ` [PATCH][RFC] ->mnt_devname is never NULL Christian Brauner
2025-04-22  7:31     ` Christian Brauner
2025-04-22 12:25       ` Al Viro
2025-04-22 13:40         ` Christian Brauner
2025-04-23  1:30         ` Al Viro
2025-04-23 22:20     ` Al Viro [this message]
2025-04-24  8:56       ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250423222045.GF2023217@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).