From dab4c96853774ba018c8a910ac8a2ec18a34c182 Mon Sep 17 00:00:00 2001
From: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
Date: Tue, 25 Jun 2019 13:59:13 -0700
Subject: [PATCH] [LibOS] Add special handling in open() syscall (EISDIR,
 O_TRUNC)

- Add error case: fail with EISDIR if directory is open()'ed with
  writable access.
- Add additional checks: O_TRUNC must work only on regular writable
  files (otherwise do not truncate).

The commit also improves LibOS regression test on FS corner cases and
fixes a tiny bug in DkStreamSetLength() with assert hanging Graphene.
---
 LibOS/shim/src/fs/shim_namei.c                   | 15 +++++++++++++--
 ...30_fs_longpath.py => 30_fopen_cornercases.py} |  4 ++--
 .../{fs_long_path.c => fopen_cornercases.c}      | 16 ++++++++++++----
 Pal/src/db_streams.c                             |  6 +++---
 4 files changed, 30 insertions(+), 11 deletions(-)
 rename LibOS/shim/test/regression/{30_fs_longpath.py => 30_fopen_cornercases.py} (70%)
 rename LibOS/shim/test/regression/{fs_long_path.c => fopen_cornercases.c} (77%)

diff --git a/LibOS/shim/src/fs/shim_namei.c b/LibOS/shim/src/fs/shim_namei.c
index 53aaf614..c27cabfb 100644
--- a/LibOS/shim/src/fs/shim_namei.c
+++ b/LibOS/shim/src/fs/shim_namei.c
@@ -499,6 +499,13 @@ int open_namei (struct shim_handle * hdl, struct shim_dentry * start,
     // lookup the path from start, passing flags
     err = __path_lookupat(start, path, lookup_flags, &mydent, 0, NULL, 0);
 
+    if (mydent && (mydent->state & DENTRY_ISDIRECTORY)) {
+        if (flags & O_WRONLY || flags & O_RDWR) {
+            err = -EISDIR;
+            goto out;
+        }
+    }
+
     // Deal with O_CREAT, O_EXCL, but only if we actually got a valid prefix
     // of directories.
     if (mydent && err == -ENOENT && (flags & O_CREAT)) {
@@ -630,8 +637,12 @@ int dentry_open (struct shim_handle * hdl, struct shim_dentry * dent,
     }
     qstrsetstr(&hdl->path, path, size);
 
-    /* truncate the file if O_TRUNC is given */
-    if (flags & O_TRUNC) {
+    /* truncate regular writable file if O_TRUNC is given */
+    if ((flags & O_TRUNC) &&
+            ((flags & O_RDWR) | (flags & O_WRONLY)) &&
+            !(dent->state & DENTRY_ISDIRECTORY) &&
+            !(dent->state & DENTRY_MOUNTPOINT) &&
+            !(dent->state & DENTRY_ISLINK)) {
         if (!fs->fs_ops->truncate) {
             ret = -EINVAL;
             goto out;
diff --git a/LibOS/shim/test/regression/30_fs_longpath.py b/LibOS/shim/test/regression/30_fopen_cornercases.py
similarity index 70%
rename from LibOS/shim/test/regression/30_fs_longpath.py
rename to LibOS/shim/test/regression/30_fopen_cornercases.py
index d0f8eec5..bdf1f336 100644
--- a/LibOS/shim/test/regression/30_fs_longpath.py
+++ b/LibOS/shim/test/regression/30_fopen_cornercases.py
@@ -4,9 +4,9 @@ from regression import Regression
 loader = sys.argv[1]
 
 # Running Long Filepath Example
-regression = Regression(loader, "fs_long_path")
+regression = Regression(loader, "fopen_cornercases")
 
-regression.add_check(name="FS Long Paths",
+regression.add_check(name="fopen corner cases",
         check=lambda res: "Successfully read from file: Hello World" in res[0].out)
 
 rv = regression.run_checks()
diff --git a/LibOS/shim/test/regression/fs_long_path.c b/LibOS/shim/test/regression/fopen_cornercases.c
similarity index 77%
rename from LibOS/shim/test/regression/fs_long_path.c
rename to LibOS/shim/test/regression/fopen_cornercases.c
index 3c1b46d6..cb83cb85 100644
--- a/LibOS/shim/test/regression/fs_long_path.c
+++ b/LibOS/shim/test/regression/fopen_cornercases.c
@@ -1,6 +1,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #define FILENAME_MAX_LENGTH 255
 #define PATH "tmp/"
@@ -19,8 +20,16 @@ int main(int argc, char** argv) {
 
     printf("filepath = %s (len = %lu)\n", filepath, strlen(filepath));
 
+    /* sanity check: try fopening dir in write mode (must fail) */
+    errno = 0;
+    FILE* fp = fopen(PATH, "w");
+    if (fp != NULL || errno != EISDIR) {
+        perror("(sanity check) fopen of dir with write access did not fail");
+        return 1;
+    }
+
     /* write to file */
-    FILE* fp = fopen(filepath, "w");
+    fp = fopen(filepath, "w");
     if (fp == NULL) {
         perror("fopen failed");
         return 1;
@@ -46,9 +55,8 @@ int main(int argc, char** argv) {
     }
 
     char buf[256];
-    clearerr(fp);
-    rets = fread(buf, sizeof(buf), 1, fp);
-    if (rets != 1 && ferror(fp)) {
+    rets = fread(buf, 1, sizeof(buf), fp);
+    if (rets != sizeof(MSG)) {
         perror("fread failed");
         return 1;
     }
diff --git a/Pal/src/db_streams.c b/Pal/src/db_streams.c
index 444fca75..ba64312a 100644
--- a/Pal/src/db_streams.c
+++ b/Pal/src/db_streams.c
@@ -658,11 +658,11 @@ DkStreamSetLength (PAL_HANDLE handle, PAL_NUM length)
     if (ret < 0) {
         _DkRaiseFailure(-ret);
         rv = -ret;
+    } else {
+        // At this point, ret should equal length
+        assert((uint64_t)ret == length);
     }
 
-    // At this point, ret should equal length
-    assert((uint64_t)ret == length);
-
     LEAVE_PAL_CALL_RETURN(rv);
 }