lib/vfscore: Fix multiple issues with dup3

dup3() is broken in multiple ways:
1) the fget() for newfd leaks the the file reference before the close
2) the error code from close() is -1 instead of the errno value
3) the old file descriptor is duplicated to a new fd slot. However, its
reference is not incremented due to the fdrop().
4) there is a race condition between the close() of newfd and its
reservation for the to-be-duplicated file description

This commit fixes all these issues except the race condition, which
needs new functionality to atomically replace a file description in
the fd tab.

Signed-off-by: Marc Rittinghaus <marc.rittinghaus@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #865

diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c
index 186f353b5a7a80015bc5b218766adb648ec1fffd..b6a09ba57852c1e1cb547e9736020f0185a2f5e6 100644 (file)
--- a/lib/vfscore/main.c
+++ b/lib/vfscore/main.c
@@ -1938,7 +1938,7 @@ UK_TRACEPOINT(trace_vfs_dup3_err, "%d", int);
  */
 UK_SYSCALL_R_DEFINE(int, dup3, int, oldfd, int, newfd, int, flags)
 {
-       struct vfscore_file *fp, *fp_new;
+       struct vfscore_file *fp;
        int error;
 
        trace_vfs_dup3(oldfd, newfd, flags);
@@ -1960,33 +1960,27 @@ UK_SYSCALL_R_DEFINE(int, dup3, int, oldfd, int, newfd, int, flags)
        if (error)
                goto out_error;
 
-       error = fget(newfd, &fp_new);
-       if (error == 0) {
-               /* if newfd is open, then close it */
-               error = close(newfd);
-               if (error)
-                       goto out_error;
-       }
+       /* BUG: The close and reserve operation must be atomic */
+       error = uk_syscall_r_close(newfd);
+       if (error && error != -EBADF)
+               goto out_error_drop;
 
        error = vfscore_reserve_fd(newfd);
        if (error)
-               goto out_error;
+               goto out_error_drop;
 
        error = vfscore_install_fd(newfd, fp);
-       if (error) {
-               fdrop(fp);
-               goto out_error;
-       }
+       if (error)
+               goto out_error_drop;
 
-       fdrop(fp);
        trace_vfs_dup3_ret(newfd);
        return newfd;
 
-       out_error:
+out_error_drop:
+       fdrop(fp);
+out_error:
        trace_vfs_dup3_err(error);
-       if(error > 0)
-               return -error;
-       return error;
+       return (error > 0) ? -error : error;
 }
 
 UK_SYSCALL_R_DEFINE(int, dup2, int, oldfd, int, newfd)