Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

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

Merged
merged 1 commit into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions LibOS/shim/src/fs/shim_namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
19 changes: 18 additions & 1 deletion LibOS/shim/test/regression/mkfifo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion LibOS/shim/test/regression/test_libos.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down