linux-kernel - Re: [PATCH] vfs: relax linkat() AT_EMPTY_PATH - aka flink()

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: <CAHk-=wjrPDx=f5OSnQVbbJ4id6SZk-exB1VW9Uz3R7rKFvTQeQ@mail.gmail.com>
Date: Thu, 11 Apr 2024 09:15:10 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Christian Brauner <brauner@...nel.org>, Charles Mirabile <cmirabil@...hat.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Andrew Lutomirski <luto@...nel.org>, Peter Anvin <hpa@...or.com>
Subject: Re: [PATCH] vfs: relax linkat() AT_EMPTY_PATH - aka flink() - requirements

On Thu, 11 Apr 2024 at 02:05, Christian Brauner <brauner@...nel.org> wrote:
>
> I had a similar discussion a while back someone requested that we relax
> permissions so linkat can be used in containers.

Hmm.

Ok, that's different - it just wants root to be able to do it, but
"root" being just in the container itself.

I don't think that's all that useful - I think one of the issues with
linkat(AT_EMPTY_PATH) is exactly that "it's only useful for root",
which means that it's effectively useless. Inside a container or out.

Because very few loads run as root-only (and fewer still run with any
capability bits that aren't just "root or nothing").

Before I did all this, I did a Debian code search for linkat with
AT_EMPTY_PATH, and it's almost non-existent. And I think it's exactly
because of this "when it's only useful for root, it's hardly useful at
all" issue.

(Of course, my Debian code search may have been broken).

So I suspect your special case is actually largely useless, and what
the container user actually wanted was what my patch does, but they
didn't think that was possible, so they asked to just extend the
"root" notion.

I've added Charles to the Cc.

But yes, with my patch, it would now be trivial to make that

        capable(CAP_DAC_READ_SEARCH)

test also be

        ns_capable(f.file->f_cred->user_ns, CAP_DAC_READ_SEARCH)

instead. I suspect not very many would care any more, but it does seem
conceptually sensible.

As to your patch - I don't like your nd->root  games in that patch at
all. That looks odd.

Yes, it makes lookup ignore the dfd (so you avoid the TOCTOU issue),
but it also makes lookup ignore "/". Which happens to be ok with an
empty path, but still...

So it feels to me like that patch of yours mis-uses something that is
just meant for vfs_path_lookup().

It may happen to work, but it smells really odd to me.

             Linus