[LibOS] Fix a crash after reusing dentry touched by mknod

This fixes a crash where we create a named pipe, remove it, and then try
to create a normal file with the same name. Unfortunately, fixing the
underlying problem (dentry reuse) is more problematic, so this is only a
workaround for that specific case.

Signed-off-by: Paweł Marczewski <[email protected]>

LibOS/shim/src/fs/shim_namei.c

@@ -58,6 +58,24 @@ static struct shim_dentry* lookup_dcache_or_create(struct shim_dentry* parent, c
     return dent;
 }
 
+/*
+ * HACK: Reset dentry state, in case this is a dentry for which we changed `fs` earlier.
+ *
+ * This is a workaround for a situation in which we create a special node (named pipe or socket) and
+ * then delete or rename it. Currently that marks the old dentry as `DENTRY_NEGATIVE` and allows it
+ * to be reused later, despite changed `fs` field.
+ *
+ * Ideally, we should always start with a fresh dentry, which would ensure all fields are in a valid
+ * state. However, that requires fixing the memory leak of `dent->data` first (see `free_dentry()`
+ * in `shim_dcache.c`).
+ */
+static void reset_dentry(struct shim_dentry* dent) {
+    if (dent->fs != dent->mount->fs) {
+        dent->fs = dent->mount->fs;
+        dent->data = NULL;
+    }
+}
+
 /*!
  * \brief Validate a dentry, if necessary
  *
@@ -82,6 +100,8 @@ static int validate_dentry(struct shim_dentry* dent) {
     if (dent->state & DENTRY_VALID)
         return 0;
 
+    reset_dentry(dent);
+
     /* This is an invalid dentry: either we just created it, or it got left over from a previous
      * failed lookup. Perform the lookup. */
     assert(dent->fs);
@@ -559,6 +579,8 @@ int open_namei(struct shim_handle* hdl, struct shim_dentry* start, const char* p
         if (ret < 0)
             goto err;
 
+        reset_dentry(dent);
+
         /* Create directory or file, depending on O_DIRECTORY. Return -EINVAL if the operation is
          * not supported for this filesystem. */
         if (flags & O_DIRECTORY) {

LibOS/shim/test/regression/mkfifo.c

@@ -9,7 +9,7 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-#define FIFO_PATH "fifo123"
+#define FIFO_PATH "tmp/fifo"
 
 int main(int argc, char** argv) {
     int fd;
@@ -93,6 +93,23 @@ int main(int argc, char** argv) {
             perror("[parent] unlink error");
             return 1;
         }
+
+        /* Check if we can create a normal file with the same name. */
+        fd = open(FIFO_PATH, O_CREAT | O_TRUNC, 0600);
+        if (fd < 0) {
+            perror("[parent] open error");
+            return 1;
+        }
+        if (close(fd) < 0) {
+            perror("[parent] close error");
+            return 1;
+        }
+        if (unlink(FIFO_PATH) < 0) {
+            perror("[parent] unlink error");
+            return 1;
+        }
+
+        printf("[parent] TEST OK\n");
     }
 
     return 0;

LibOS/shim/test/regression/test_libos.py

@@ -874,8 +874,13 @@ def test_092_pipe_ocloexec(self):
         self.assertIn('TEST OK', stdout)
 
     def test_095_mkfifo(self):
-        stdout, _ = self.run_binary(['mkfifo'], timeout=60)
+        try:
+            stdout, _ = self.run_binary(['mkfifo'], timeout=60)
+        finally:
+            if os.path.exists('tmp/fifo'):
+                os.remove('tmp/fifo')
         self.assertIn('read on FIFO: Hello from write end of FIFO!', stdout)
+        self.assertIn('[parent] TEST OK', stdout)
 
     def test_100_socket_unix(self):
         stdout, _ = self.run_binary(['unix'])