Re: [PATCH RFC 4/7] fs: support FD_FAILFS_ROOT in fchroot()

On 2026-07-23 06:50 -0700, Andy Lutomirski wrote:
> On Thu, Jul 23, 2026 at 6:09 AM Christian Brauner <[email protected]> wrote:
> >
> > On Thu, Jul 23, 2026 at 05:49:06AM -0700, Andy Lutomirski wrote:
> > > On Thu, Jul 23, 2026 at 4:37 AM Christian Brauner <[email protected]> wrote:
> > > >
> > > > Once entered, failfs is a throw-away-the-key moment. The task is
> > > > considered chrooted so it cannot create user namespaces to regain
> > > > CAP_SYS_CHROOT. chroot()/fchroot() back out require CAP_SYS_CHROOT.
> > >
> > > I kind of alluded to this earlier, but I don't think we should promise
> > > this.  In particular, I still think we should eventually allow a
> > > non-chrooted process with no_new_privs to chroot to any valid
> > > directory, and I think we should consider changing the definition of
> > > current_chrooted() such that failfs-as-root would cause it to return
> > > false.  I'm also not sure why it's useful -- if I want to throw away
> >
> > I had considered that but introducing this change alongside this series
> > felt too spicy for me.
> 
> I think I agree.  My only actual objection to your series as is is
> that your wording seems to make a promise that I think shouldn't be
> made.

Agreed.

> 
> >
> > > the key to accessing the normal contents of my mountns without
> > > changing my mountns, I need to chroot somewhere (like failfs) *and* I
> > > need to somehow prevent myself from getting an fd or a magic link back
> > > to the ordinary mountns contents.  If I somehow get such an fd,
> > > preventing my from fchrooting to it doesn't seem to accomplish
> > > anything, since I could traverse the fd with openat, etc just as
> > > easily, or even fchdir to it and use relative paths.
> >
> > I think I generally agree with you. My main concern here is mostly about
> > changing very long-standing behavior here and making failfs special here
> > feels like it could easily misused.
> 
> I don't think either of us are really suggesting making failfs
> special.  I agree about the long-standing behavior.
> 
> >
> > >
> > > My understanding of the exact workings of the mount tree is possibly
> > > not good enough to specify the semantics I think are correct quite
> > > exactly, but I think a decent approximation is that a task should be
> > > considered to be not chrooted if its root is at the top of its mount
> > > tree or is not in a mount tree at all.
> >
> > If we could altering this definition from the patchset I would be in
> > favor of it. I'm curious what your take is though.
> >
> 
> I can't quite parse your question.

Maybe I should shunt everything I write through an LLM so it catches
grammatical nonsense.

It was basically just trying to say "let's not redesign
current_chrooted()" in this series - which I already did above.

> I have three sort-of-concrete proposals:

The best proposals...

> (a) current_chrooted returns false if follow_dotdot starting at
> current root but with nd->root equal to the namespace root would
> succeed and return the current root.  I think this is fairly solid.
> The main caveat I can think of is that someone could create a detached
> mount tree, chroot to its root, then fork and have the child:
> 
>  - drop privileges
>  - set no_new_privs
>  - chroot somewhere else
> 
> then the parent or another privileged task mounts that tree somewhere,
> and the child could now access the parent of the mountpoint.  Any
> actual exploit based on this seems farfetched.

It would also be easy to prevent unprivileged chroot()ing into detached
mount trees. They're easy to recognize and it's not a super common
use-case.

> 
> (b) separate the concepts of fs-root-for-absolute-paths and
> fs-root-for-blocking-dotdot.  IMO this actually makes sense but would
> would be a much more intrusive change.  We would have
> fs->root_of_abspaths and fs->dotdot_blocking_root, and fchroot would
> only change root_of_abspaths unless a flag is set asking to change
> both.  (And document that setting that flag is not advised and that
> one should create a detached mount tree instead for 90% of use cases.)
>  And no_new_privs tasks would be allowed to freely change
> root_of_abspaths.
> 
> I like (b) conceptually in the sense that I wish no one had ever
> invented chroot-for-security in the first place.  I'm not sure I like
> it as an actual practical idea because it has a much larger scope.
> Also (a) is almost equivalent as long as no one ever sets the root to
> something that isn't the root of a mount tree, and it really would
> confuse some people if "/.." was not equivalent to "./".

Yeah, I'm not so excited about this part...

> 
> (c) Add a bit to fs_struct called something like chroot_locked.
> chroot() sets the bit if it chroots anywhere other than "/" or its
> equivalent.  fchroot does not set it unless a flag asking for it is
> set.  Switching mount namespace clears the bit.  If you have
> no_new_privs and don't have chroot_locked then you can chroot.

That sounds reasonable - the fchroot() extension is also a nice touch.