All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: Al Airy <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org,
	 Linus Torvalds <torvalds@linux-foundation.org>,
	 Christian Brauner <brauner@kernel.org>
Subject: Re: [RFC] MNT_LOCKED vs. finish_automount()
Date: Fri, 02 May 2025 22:46:26 -0500	[thread overview]
Message-ID: <87plgq8igd.fsf@email.froward.int.ebiederm.org> (raw)
In-Reply-To: <20250501201506.GS2023217@ZenIV> (Al Viro's message of "Thu, 1 May 2025 21:15:06 +0100")

Al Viro <viro@zeniv.linux.org.uk> writes:

> 	Back in 2011, when ->d_automount() had been introduced,
> we went with "stepping on NFS referral, etc., has the submount
> inherit the flags of parent one" (with the obvious exceptions
> for internal-only flags).  Back then MNT_LOCKED didn't exist.
>
> 	Two years later, when MNT_LOCKED had been added, an explicit
> "don't set MNT_LOCKED on expirable mounts when propagating across
> the userns boundary; their underlying mountpoints can be exposed
> whenever the original expires anyway".  Same went for root of
> subtree attached by explicit mount --[r]bind - the mountpoint
> had been exposed before the call, after all and for roots of
> any propagation copies created by such (same reason).  Normal mount
> (created by do_new_mount()) could never get MNT_LOCKED to start with.
>
> 	However, mounts created by finish_automount() bloody well
> could - if the parent mount had MNT_LOCKED on it, submounts would
> inherited it.  Even if they had been expirable.  Moreover, all their
> propagation copies would have MNT_LOCKED stripped out.
>
> 	IMO this inconsistency is a bug; MNT_LOCKED should not
> be inherited in finish_automount().
>
> 	Eric, is there something subtle I'm missing here?

I don't think you are missing anything.  This looks like a pretty clear
cut case of simply not realizing finish_automount was special in a way
that could result in MNT_LOCKED getting set.

I skimmed through the code just a minute ago and my reading of it
matches your reading of it above.

The intended semantics of MNT_LOCKED are to not let an unprivileged user
see under mounts they would never be able to see under without creating
a mount namespace.

The mount point of an automount is pretty clearly something that is safe
to see under.  Doubly so if this is a directory that will always be
empty on a pseudo filesystem (aka autofs).

Eric

  reply	other threads:[~2025-05-03  4:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-01 20:15 [RFC] MNT_LOCKED vs. finish_automount() Al Viro
2025-05-03  3:46 ` Eric W. Biederman [this message]
2025-05-04 23:24   ` Al Viro
2025-05-05 13:52     ` Eric W. Biederman
2025-05-05 19:02       ` Al Viro
2025-05-06  2:25         ` Al Viro
2025-05-05 13:54     ` 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=87plgq8igd.fsf@email.froward.int.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.