Fix dup() of 9ns FDs
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 16 Oct 2017 17:49:57 +0000 (13:49 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Mon, 16 Oct 2017 17:49:57 +0000 (13:49 -0400)
Glibc uses dup() to implement dup2(), by way of sysdeps/posix/dup2.c.  VFS
dup() was handling the low_fd argument, but 9ns wasn't.  The fix is to
propagate the low_fd (and must_use_low) down through newfd() to
insert_obj_fdt().

Note that fcntl's dup() doesn't force us to use the fd - it's a hint.  The
bigger issue is that glibc's implementation of dup2 might fail; a
concurrent thread could open newfd.  If this is an issue, we'll need an
in-kernel version of dup2().

Fixes brho/akaros#41.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ns.h
kern/src/ns/sysfile.c
kern/src/syscall.c
user/utest/file-posix.c

index a505011..b5ff1dd 100644 (file)
@@ -942,7 +942,7 @@ void cmderror(struct cmdbuf *cb, char *s);
 struct cmdtab *lookupcmd(struct cmdbuf *cb, struct cmdtab *ctab, int nctab);
 
 /* kern/src/ns/sysfile.c */
-int newfd(struct chan *c, int oflags);
+int newfd(struct chan *c, int low_fd, int oflags, bool must_use_low);
 struct chan *fdtochan(struct fd_table *fdt, int fd, int mode, int chkmnt,
                       int iref);
 long kchanio(void *vc, void *buf, int n, int mode);
@@ -952,7 +952,7 @@ int syschdir(char *path);
 int grpclose(struct fd_table *fdt, int fd);
 int sysclose(int fd);
 int syscreate(char *path, int mode, uint32_t perm);
-int sysdup(int old);
+int sysdup(int old, int low_fd, bool must_use_low);
 int sys_dup_to(struct proc *from_proc, unsigned int from_fd,
                struct proc *to_proc, unsigned int to_fd);
 int sysfstat(int fd, uint8_t*, int n);
index 16cd623..90c36f6 100644 (file)
@@ -48,11 +48,11 @@ enum {
                                 * let's not yet exceed a common MSIZE. */
 };
 
-int newfd(struct chan *c, int oflags)
+int newfd(struct chan *c, int low_fd, int oflags, bool must_use_low)
 {
-       int ret = insert_obj_fdt(&current->open_files, c, 0,
+       int ret = insert_obj_fdt(&current->open_files, c, low_fd,
                                 oflags & O_CLOEXEC ? FD_CLOEXEC : 0,
-                                FALSE, FALSE);
+                                must_use_low, FALSE);
        if (ret >= 0)
                cclose(c);
        return ret;
@@ -199,7 +199,8 @@ int syscreate(char *path, int mode, uint32_t perm)
                cclose(c);
                nexterror();
        }
-       fd = newfd(c, mode);    /* 9ns mode is the O_FLAGS and perm is glibc mode */
+       /* 9ns mode is the O_FLAGS and perm is glibc mode */
+       fd = newfd(c, 0, mode, FALSE);
        if (fd < 0)
                error(-fd, ERROR_FIXME);
        poperror();
@@ -208,7 +209,7 @@ int syscreate(char *path, int mode, uint32_t perm)
        return fd;
 }
 
-int sysdup(int old)
+int sysdup(int old, int low_fd, bool must_use_low)
 {
        ERRSTACK(1);
        int fd;
@@ -223,7 +224,7 @@ int sysdup(int old)
                cclose(c);
                error(EPERM, ERROR_FIXME);
        }
-       fd = newfd(c, 0);
+       fd = newfd(c, low_fd, 0, must_use_low);
        if (fd < 0) {
                cclose(c);
                error(-fd, ERROR_FIXME);
@@ -313,7 +314,7 @@ int sysfauth(int fd, char *aname)
                nexterror();
        }
 
-       fd = newfd(ac, 0);
+       fd = newfd(ac, 0, 0, FALSE);
        if (fd < 0)
                error(-fd, ERROR_FIXME);
        poperror();     /* ac */
@@ -526,7 +527,7 @@ int sysopenat(int fromfd, char *path, int vfs_flags)
                        error(EINVAL, "Cannot openat from a non-O_PATH FD");
                c = namec_from(from, path, Aopen, vfs_flags, 0);
        }
-       fd = newfd(c, vfs_flags);
+       fd = newfd(c, 0, vfs_flags, FALSE);
        if (fd < 0)
                error(-fd, ERROR_FIXME);
        poperror();
index fe3538a..d0cfb1e 100644 (file)
@@ -1867,7 +1867,13 @@ intreg_t sys_fcntl(struct proc *p, int fd, int cmd, unsigned long arg1,
                /* 9ns hack */
                switch (cmd) {
                        case (F_DUPFD):
-                               return sysdup(fd);
+                               newfd = arg1;
+                               if (newfd < 0) {
+                                       set_errno(EBADF);
+                                       return -1;
+                               }
+                               /* TODO: glibc uses regular DUPFD for dup2, which is racy. */
+                               return sysdup(fd, newfd, FALSE);
                        case (F_GETFD):
                        case (F_SETFD):
                        case (F_SYNC):
@@ -1889,7 +1895,12 @@ intreg_t sys_fcntl(struct proc *p, int fd, int cmd, unsigned long arg1,
        /* TODO: these are racy */
        switch (cmd) {
                case (F_DUPFD):
-                       retval = insert_file(&p->open_files, file, arg1, FALSE, FALSE);
+                       newfd = arg1;
+                       if (newfd < 0) {
+                               set_errno(EBADF);
+                               return -1;
+                       }
+                       retval = insert_file(&p->open_files, file, newfd, FALSE, FALSE);
                        if (retval < 0) {
                                set_errno(-retval);
                                retval = -1;
index f4ff473..1aab4fe 100644 (file)
@@ -56,11 +56,30 @@ bool test_open_lots_and_spawn(void)
        return TRUE;
 }
 
+bool test_pipe_dup2(void)
+{
+       int pipefd[2];
+       int ret;
+
+       ret = pipe(pipefd);
+       UT_ASSERT(!ret);
+       ret = dup2(pipefd[0], 100);
+       UT_ASSERT(ret == 100);
+       ret = close(100);
+       UT_ASSERT(!ret);
+       ret = close(pipefd[0]);
+       UT_ASSERT(!ret);
+       ret = close(pipefd[1]);
+       UT_ASSERT(!ret);
+       return TRUE;
+}
+
 /* <--- End definition of test cases ---> */
 
 struct utest utests[] = {
        UTEST_REG(openat),
        UTEST_REG(open_lots_and_spawn),
+       UTEST_REG(pipe_dup2),
 };
 int num_utests = sizeof(utests) / sizeof(struct utest);