Skip to content

Commit 42bd2af

Browse files
torvaldsbrauner
authored andcommitted
vfs: relax linkat() AT_EMPTY_PATH - aka flink() - requirements
"The definition of insanity is doing the same thing over and over again and expecting different results” We've tried to do this before, most recently with commit bb2314b ("fs: Allow unprivileged linkat(..., AT_EMPTY_PATH) aka flink") about a decade ago. But the effort goes back even further than that, eg this thread back from 1998 that is so old that we don't even have it archived in lore: https://lkml.org/lkml/1998/3/10/108 which also points out some of the reasons why it's dangerous. Or, how about then in 2003: https://lkml.org/lkml/2003/4/6/112 where we went through some of the same arguments, just wirh different people involved. In particular, having access to a file descriptor does not necessarily mean that you have access to the path that was used for lookup, and there may be very good reasons why you absolutely must not have access to a path to said file. For example, if we were passed a file descriptor from the outside into some limited environment (think chroot, but also user namespaces etc) a 'flink()' system call could now make that file visible inside a context where it's not supposed to be visible. In the process the user may also be able to re-open it with permissions that the original file descriptor did not have (eg a read-only file descriptor may be associated with an underlying file that is writable). Another variation on this is if somebody else (typically root) opens a file in a directory that is not accessible to others, and passes the file descriptor on as a read-only file. Again, the access to the file descriptor does not imply that you should have access to a path to the file in the filesystem. So while we have tried this several times in the past, it never works. The last time we did this, that commit bb2314b quickly got reverted again in commit f0cc6ff (Revert "fs: Allow unprivileged linkat(..., AT_EMPTY_PATH) aka flink"), with a note saying "We may re-do this once the whole discussion about the interface is done". Well, the discussion is long done, and didn't come to any resolution. There's no question that 'flink()' would be a useful operation, but it's a dangerous one. However, it does turn out that since 2008 (commit d76b0d9: "CRED: Use creds in file structs") we have had a fairly straightforward way to check whether the file descriptor was opened by the same credentials as the credentials of the flink(). That allows the most common patterns that people want to use, which tend to be to either open the source carefully (ie using the openat2() RESOLVE_xyz flags, and/or checking ownership with fstat() before linking), or to use O_TMPFILE and fill in the file contents before it's exposed to the world with linkat(). But it also means that if the file descriptor was opened by somebody else, or we've gone through a credentials change since, the operation no longer works (unless we have CAP_DAC_READ_SEARCH capabilities in the opener's user namespace, as before). Note that the credential equality check is done by using pointer equality, which means that it's not enough that you have effectively the same user - they have to be literally identical, since our credentials are using copy-on-write semantics. So you can't change your credentials to something else and try to change it back to the same ones between the open() and the linkat(). This is not meant to be some kind of generic permission check, this is literally meant as a "the open and link calls are 'atomic' wrt user credentials" check. It also means that you can't just move things between namespaces, because the credentials aren't just a list of uid's and gid's: they includes the pointer to the user_ns that the capabilities are relative to. So let's try this one more time and see if maybe this approach ends up being workable after all. Cc: Andrew Lutomirski <[email protected]> Cc: Al Viro <[email protected]> Cc: Christian Brauner <[email protected]> Cc: Peter Anvin <[email protected]> Cc: Jan Kara <[email protected]> Signed-off-by: Linus Torvalds <[email protected]> Link: https://lore.kernel.org/r/[email protected] [brauner: relax capability check to opener of the file] Link: https://lore.kernel.org/all/20231113-undenkbar-gediegen-efde5f1c34bc@brauner Signed-off-by: Christian Brauner <[email protected]>
1 parent fd0a133 commit 42bd2af

File tree

2 files changed

+14
-6
lines changed

2 files changed

+14
-6
lines changed

fs/namei.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,6 +2422,14 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
24222422
if (!f.file)
24232423
return ERR_PTR(-EBADF);
24242424

2425+
if (flags & LOOKUP_LINKAT_EMPTY) {
2426+
if (f.file->f_cred != current_cred() &&
2427+
!ns_capable(f.file->f_cred->user_ns, CAP_DAC_READ_SEARCH)) {
2428+
fdput(f);
2429+
return ERR_PTR(-ENOENT);
2430+
}
2431+
}
2432+
24252433
dentry = f.file->f_path.dentry;
24262434

24272435
if (*s && unlikely(!d_can_lookup(dentry))) {
@@ -4644,14 +4652,13 @@ int do_linkat(int olddfd, struct filename *old, int newdfd,
46444652
goto out_putnames;
46454653
}
46464654
/*
4647-
* To use null names we require CAP_DAC_READ_SEARCH
4655+
* To use null names we require CAP_DAC_READ_SEARCH or
4656+
* that the open-time creds of the dfd matches current.
46484657
* This ensures that not everyone will be able to create
4649-
* handlink using the passed filedescriptor.
4658+
* a hardlink using the passed file descriptor.
46504659
*/
4651-
if (flags & AT_EMPTY_PATH && !capable(CAP_DAC_READ_SEARCH)) {
4652-
error = -ENOENT;
4653-
goto out_putnames;
4654-
}
4660+
if (flags & AT_EMPTY_PATH)
4661+
how |= LOOKUP_LINKAT_EMPTY;
46554662

46564663
if (flags & AT_SYMLINK_FOLLOW)
46574664
how |= LOOKUP_FOLLOW;

include/linux/namei.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
4444
#define LOOKUP_BENEATH 0x080000 /* No escaping from starting point. */
4545
#define LOOKUP_IN_ROOT 0x100000 /* Treat dirfd as fs root. */
4646
#define LOOKUP_CACHED 0x200000 /* Only do cached lookup */
47+
#define LOOKUP_LINKAT_EMPTY 0x400000 /* Linkat request with empty path. */
4748
/* LOOKUP_* flags which do scope-related checks based on the dirfd. */
4849
#define LOOKUP_IS_SCOPED (LOOKUP_BENEATH | LOOKUP_IN_ROOT)
4950

0 commit comments

Comments
 (0)