lib/vfscore: Enable FIONBIO in fcntl(F_SETFL)

Currently, we are synching FIONBIO to the underlying file
implementation via an ioctl only if FIONBIO is defined in
vfscore. Since this is not the case, setting O_NONBLOCK on
a file descriptor does not have any effect. This commit
includes the necessary header and also fixes the
fcntl(F_SETFL) and ioctl() to make sure that the state of
the file stays in sync with its flags.

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

diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c
index 65fea637d239004ff0599e183bc13b867b0bea25..186f353b5a7a80015bc5b218766adb648ec1fffd 100644 (file)
--- a/lib/vfscore/main.c
+++ b/lib/vfscore/main.c
@@ -34,6 +34,7 @@
 
 #include <sys/statvfs.h>
 #include <sys/stat.h>
+#include <sys/ioctl.h>
 #include <limits.h>
 #include <string.h>
 #include <stdlib.h>
@@ -2009,9 +2010,7 @@ UK_LLSYSCALL_R_DEFINE(int, fcntl, int, fd, unsigned int, cmd, int, arg)
 {
        struct vfscore_file *fp;
        int ret = 0, error;
-#if defined(FIONBIO) && defined(FIOASYNC)
-       int tmp;
-#endif
+       int tmp, oldf;
 
        trace_vfs_fcntl(fd, cmd, arg);
        error = fget(fd, &fp);
@@ -2048,17 +2047,44 @@ UK_LLSYSCALL_R_DEFINE(int, fcntl, int, fd, unsigned int, cmd, int, arg)
                break;
        case F_SETFL:
                FD_LOCK(fp);
-               fp->f_flags = vfscore_fflags((vfscore_oflags(fp->f_flags) & ~SETFL) |
-                               (arg & SETFL));
-               FD_UNLOCK(fp);
-
-#if defined(FIONBIO) && defined(FIOASYNC)
-               /* Sync nonblocking/async state with file flags */
+               oldf = fp->f_flags;
+               tmp  = vfscore_oflags(fp->f_flags) & ~SETFL;
+               fp->f_flags = vfscore_fflags(tmp | (arg & SETFL));
+
+               /* Sync nonblocking/async state with file flags
+                * To make sure that the actual state of the underlying file
+                * is in sync with the configured flag, we need to perform the
+                * ioctl while holding the lock. This is not optimal since we
+                * hold the lock for the duration of potentially expensive
+                * operations. It would be better to just set the flag here and
+                * make sure that all components directly consume this flag
+                * instead of synching it to other places.
+                */
                tmp = fp->f_flags & FNONBLOCK;
-               vfs_ioctl(fp, FIONBIO, &tmp);
+               if ((tmp ^ oldf) & FNONBLOCK) {
+                       error = vfs_ioctl(fp, FIONBIO, &tmp);
+                       if (unlikely(error)) {
+                               fp->f_flags = oldf;
+                               FD_UNLOCK(fp);
+                               break;
+                       }
+               }
+
                tmp = fp->f_flags & FASYNC;
-               vfs_ioctl(fp, FIOASYNC, &tmp);
-#endif
+               if ((tmp ^ oldf) & FASYNC) {
+                       error = vfs_ioctl(fp, FIOASYNC, &tmp);
+                       if (unlikely(error)) {
+                               tmp = oldf & FNONBLOCK;
+                               if ((tmp ^ fp->f_flags) & FNONBLOCK)
+                                       (void)vfs_ioctl(fp, FIONBIO, &tmp);
+
+                               fp->f_flags = oldf;
+                               FD_UNLOCK(fp);
+                               break;
+                       }
+               }
+
+               FD_UNLOCK(fp);
                break;
        case F_DUPFD_CLOEXEC:
                error = fdalloc(fp, &ret);

diff --git a/lib/vfscore/syscalls.c b/lib/vfscore/syscalls.c
index 45e74b8ff683c9e85aa6e8278a14814e6873b64f..43b3df2d4743ef9a12f4a103d89e51896bf6f87b 100644 (file)
--- a/lib/vfscore/syscalls.c
+++ b/lib/vfscore/syscalls.c
@@ -382,24 +382,46 @@ int
 sys_ioctl(struct vfscore_file *fp, unsigned long request, void *buf)
 {
        int error = 0;
+       int oldf;
 
        DPRINTF(VFSDB_SYSCALL, ("sys_ioctl: fp=%p request=%lux\n", fp, request));
 
        if ((fp->f_flags & (UK_FREAD | UK_FWRITE)) == 0)
                return EBADF;
 
+       FD_LOCK(fp);
+       oldf = fp->f_flags;
+
        switch (request) {
        case FIOCLEX:
                fp->f_flags |= O_CLOEXEC;
-               break;
+               goto exit;
        case FIONCLEX:
                fp->f_flags &= ~O_CLOEXEC;
+               goto exit;
+       case FIONBIO:
+               UK_ASSERT(buf);
+               if (*(int *)buf)
+                       fp->f_flags |= FNONBLOCK;
+               else
+                       fp->f_flags &= ~FNONBLOCK;
                break;
-       default:
-               error = vfs_ioctl(fp, request, buf);
+       case FIOASYNC:
+               UK_ASSERT(buf);
+               if (*(int *)buf)
+                       fp->f_flags |= FASYNC;
+               else
+                       fp->f_flags &= ~FASYNC;
                break;
        }
 
+       error = vfs_ioctl(fp, request, buf);
+       if (unlikely(error))
+               fp->f_flags = oldf;
+
+exit:
+       FD_UNLOCK(fp);
+
        DPRINTF(VFSDB_SYSCALL, ("sys_ioctl: comp error=%d\n", error));
        return error;
 }