From fc9685031e6838450f10a6a50cdb3162268bd455 Mon Sep 17 00:00:00 2001 From: Marc Rittinghaus Date: Thu, 20 Apr 2023 09:49:02 +0200 Subject: [PATCH] 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 Reviewed-by: Andra Paraschiv Reviewed-by: Stefan Jumarea Reviewed-by: Razvan Deaconescu Approved-by: Razvan Deaconescu Tested-by: Unikraft CI GitHub-Closes: #850 --- lib/vfscore/main.c | 50 ++++++++++++++++++++++++++++++++---------- lib/vfscore/syscalls.c | 28 ++++++++++++++++++++--- 2 files changed, 63 insertions(+), 15 deletions(-) diff --git a/lib/vfscore/main.c b/lib/vfscore/main.c index 65fea637d..186f353b5 100644 --- a/lib/vfscore/main.c +++ b/lib/vfscore/main.c @@ -34,6 +34,7 @@ #include #include +#include #include #include #include @@ -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 45e74b8ff..43b3df2d4 100644 --- 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; } -- 2.39.5