Skip to contentSkip to conversation

new syscall: flink

PATCH In review Ulrich Drepper 3 participants 4 messages Apr 6, 2003 – Apr 8, 2003 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 new syscall: flink

I got a couple of requests for a function which isn't support on Linux so far. Also not supportable, i.e., cannot be emulated at userlevel. It has some history in other systems (QNX I think), though, and helps with some security issues. It really not adding much new functionality and I hope I got it right with my "monkey see, monkey do" technique of looking up other places doing similar things.

The syscall I mean is

int flink (int fd, const char *newname)

Similar to link(), but the first parameter is a file decsriptor. Using the file descriptor helps to avoid races in some situation. Look at this code bit (this is constructed and just a little test case):

#include <errno.h>
#define EE(a,x) {int e = a;if (e!=x)printf("%s = %d (%m)\n", #a, errno);} #define E(a) EE(a,0)
int
main()
{
printf("uid = %d, euid = %d, gid = %d, egid = %d\n", getuid(),
geteuid(), getgid(), getegid());
E(setfsuid(getuid()));
E(setfsgid(getgid()));
char buf[] = "aaXXXXXX";
int fd = mkstemp (buf);
EE(setfsuid(0),getuid());
EE(setfsgid(0),getgid());
E(fchown(fd,48,48));
E(setfsuid(getuid()));
E(setfsgid(getgid()));
E(syscall(268,fd,"aa"));
E(unlink(buf));
return 0;
}

This is for a SUID/SGID root application. A temporary file is created carefully using the permissions of the user running the program. If the syscall 268 (flink) line would be

link(buf,"aa)

instead, somebody with limited priviledges could in theory have unlinked the temporary file and created a new one. With flink() this isn't possible. And the best is: the unlink() line can be moved right below the mkstemp() call. This means no temporary files are around if something goes wrong before the unlink(). (For me this is at least as important as the security issue, it simplifies temp file handling and reduces the number of bugs == leftover fiels).

The patch itself is very minimal. The impact on the link() syscall is not measurable and the extra code for sys_flink is only a few bytes.

Is this acceptable? Shall I add more syscall definitions for platforms != x86 (I'd think the port maintainers want to do this themselves)?

Signature · 3 lines
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
arch/i386/kernel/entry.S renamed from arch/i386/kernel/entry.S-flink +1 -0
@@ -852,6 +852,7 @@ ENTRY(sys_call_table)
  	.long sys_clock_gettime		/* 265 */
  	.long sys_clock_getres
  	.long sys_clock_nanosleep
+	.long sys_flink
  
  
 nr_syscalls=(.-sys_call_table)/4
fs/namei.c renamed from fs/namei.c-flink +32 -10

80 lines not on this page · raw message

include/asm-i386/unistd.h renamed from include/asm-i386/unistd.h-flink +2 -1
@@ -273,8 +273,9 @@
 #define __NR_clock_gettime	(__NR_timer_create+6)
 #define __NR_clock_getres	(__NR_timer_create+7)
 #define __NR_clock_nanosleep	(__NR_timer_create+8)
+#define __NR_flink		268
 
-#define NR_syscalls 268
+#define NR_syscalls 269
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
#1 lore raw

In article <[email protected]>,

Quoted 7 lines · Ulrich Drepper

I got a couple of requests for a function which isn't support on Linux so far. Also not supportable, i.e., cannot be emulated at userlevel. It has some history in other systems (QNX I think), though, and helps with some security issues. It really not adding much new functionality and I hope I got it right with my "monkey see, monkey do" technique of looking up other places doing similar things.

As others have pointed out, there is no way in HELL we can do this securely without major other incursions.

In particular, both flink() and funlink() require that you do all the same permission checks that a real link() or unlink() would do. And as some of them are done on the _source_ of the file, that implies that they have to be done at open() time.

One check in particular is "is the opener willing to let this be linked anywhere else in the namespace". Since the opener isn't necessarily the same agent as the one doing the flink().

If you really really think you need this (and not just do it because some random idiot-customer doesn't understand security), then I would suggest you add a O_CANLINK flag to open, and require that that flag is set in the file descriptor.

That way you get "flink()" behaviour, but you require that the opener be aware of the fact that the file may be linked into another position. That will fix the glaring security hole.

Linus

#2 lore raw
Followup to:  <[email protected]>
By author:    [email protected] (Linus Torvalds)
In newsgroup: linux.dev.kernel
Quoted 4 lines

As others have pointed out, there is no way in HELL we can do this securely without major other incursions.

If so, we already have the security hole... and we need to fix it.

Quoted 4 lines

In particular, both flink() and funlink() require that you do all the same permission checks that a real link() or unlink() would do. And as some of them are done on the _source_ of the file, that implies that they have to be done at open() time.

[f]link() doesn't do any checks that open() doesn't, except for the O_RDONLY/O_WRONLY/O_RDWR flags.

-hpa

Signature · 3 lines
<[email protected]> at work, <[email protected]> in private!
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64
#3 lore raw
In reply to Linus Torvalds

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 4 lines · Linus Torvalds

If you really really think you need this (and not just do it because some random idiot-customer doesn't understand security), then I would suggest you add a O_CANLINK flag to open, and require that that flag is set in the file descriptor.

Nobody would be stupid enough to let me work on customer issues related to the kernel. The requests come in through glibc users.

Anyway, there are two or three ways I can see:

- - add safelink() instead of flink()

int safelink (const char *oldname, int fd, const char *newname)

As Jakub explained, this syscall would check that the file referenced by oldname really corresponds to fd before making the link.

If you need an example, take the linker. The linking can take a long time. The temporary output file has to be created early and there is sometimes enough time before the linking finished for even a human to figure out the temporary file's name and replace the file. If then the final file is installed as root (since the linking succeeds) you'll have problems.

There is of course the possibility that one compares the ino/dev of the temporary file and the file after the link (or rename, btw) but this means the wrong output file existed for some time and all that is left to do is to remove the file which might be critical if the system depends on the file always being present.

- - add the O_CANLINK. Sure it's possible. But see the next variant

- - add an open() flag to create files which are not present in the filesystem (Hurd has something like this). open() would get as the filename the name of a directory. Such a feature can be used for all kinds of temporary files:

+ files which never need names, i.e., don't have to be accessed through the filesystem; the advantage is that there would never be stray files in the filesystem if the program forgets to clean them up

+ temp files which have to be completed first before renamed. Here flink() and frename() would introduce the name in the filesystem. This is obviously useful in many many places, e.g., the linker scenario. There is no way to attack the linker while it is doing its work since the output file isn't visible until it is installed under the final name.

Maybe the O_CANLINK flag idea is also necessary for this, don't know. The O_ANONYMOUS flag might include setting O_CANLINK.

I'm certainly not qualified to say whether this is viable or not. The safelink() idea certainly is implementable, just 3-4 more lines on top of the flink() patch. But this wouldn't be necessary if we'd have the more complete support with the new open() flag(s). Al mentioned to me some problems with network filesystem in the context of flink(). So somebody who understands these issues might want to comment. It seems there is some interest in this.

- --
- --------------. ,-. 444 Castro Street
Ulrich Drepper \ ,-----------------' \ Mountain View, CA 94041 USA
Red Hat --' drepper at redhat.com ---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+khaN2ijCOnn/RHQRAm09AJ9Fin1Js+Qla9vVSRzawSASl0dCNACfdoU1 VxrcNGyC06OxRiQZ0+reo0k=
=PosI
-----END PGP SIGNATURE-----