Skip to content

Commit

Permalink
keep referential integrity for file table entries
Browse files Browse the repository at this point in the history
Before this, some functions such as `uvwasi_path_open()` could
crash, because they acquired a reference to a fd table entry
using `uvwasi_fd_table_get()` and continued to use the result
after calling `uvwasi_fd_table_insert_fd()`; however, the latter
could `realloc()` the fd table, potentially invalidating the
earlier return value of `uvwasi_fd_table_get()`.

Since the previous commit introduced a per-fd allocation anyway,
this modifies the code to allocate the table entry itself as part
of that allocation. This ensures referential integrity for
the table entry.

(Aside -- I’m not sure that it was valid to call `uv_mutex_init()`
and then move that mutex around in memory, either.)
  • Loading branch information
addaleax authored and cjihrig committed Dec 4, 2019
1 parent 20d20fe commit 7579e29
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 19 deletions.
3 changes: 1 addition & 2 deletions include/fd_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ struct uvwasi_fd_wrap_t {
uvwasi_rights_t rights_base;
uvwasi_rights_t rights_inheriting;
int preopen;
int valid;
uv_mutex_t mutex;
};

struct uvwasi_fd_table_t {
struct uvwasi_fd_wrap_t* fds;
struct uvwasi_fd_wrap_t** fds;
uint32_t size;
uint32_t used;
uv_rwlock_t rwlock;
Expand Down
38 changes: 21 additions & 17 deletions src/fd_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ static uvwasi_errno_t uvwasi__fd_table_insert(uvwasi_t* uvwasi,
int preopen,
struct uvwasi_fd_wrap_t** wrap) {
struct uvwasi_fd_wrap_t* entry;
struct uvwasi_fd_wrap_t* new_fds;
struct uvwasi_fd_wrap_t** new_fds;
uvwasi_errno_t err;
uint32_t new_size;
int index;
Expand All @@ -200,8 +200,11 @@ static uvwasi_errno_t uvwasi__fd_table_insert(uvwasi_t* uvwasi,

mp_len = strlen(mapped_path);
rp_len = strlen(real_path);
mp_copy = uvwasi__malloc(uvwasi, mp_len + rp_len + 2);
if (mp_copy == NULL) return UVWASI_ENOMEM;
entry = (struct uvwasi_fd_wrap_t*)
uvwasi__malloc(uvwasi, sizeof(*entry) + mp_len + rp_len + 2);
if (entry == NULL) return UVWASI_ENOMEM;

mp_copy = (char*)(entry + 1);
rp_copy = mp_copy + mp_len + 1;
memcpy(mp_copy, mapped_path, mp_len);
mp_copy[mp_len] = '\0';
Expand All @@ -215,12 +218,13 @@ static uvwasi_errno_t uvwasi__fd_table_insert(uvwasi_t* uvwasi,
new_size = table->size * 2;
new_fds = uvwasi__realloc(uvwasi, table->fds, new_size * sizeof(*new_fds));
if (new_fds == NULL) {
uvwasi__free(uvwasi, entry);
err = UVWASI_ENOMEM;
goto exit;
}

for (i = table->size; i < new_size; ++i)
new_fds[i].valid = 0;
new_fds[i] = NULL;

index = table->size;
table->fds = new_fds;
Expand All @@ -229,20 +233,21 @@ static uvwasi_errno_t uvwasi__fd_table_insert(uvwasi_t* uvwasi,
/* The table is big enough, so find an empty slot for the new data. */
index = -1;
for (i = 0; i < table->size; ++i) {
if (table->fds[i].valid != 1) {
if (table->fds[i] == NULL) {
index = i;
break;
}
}

/* index should never be -1. */
if (index == -1) {
uvwasi__free(uvwasi, entry);
err = UVWASI_ENOSPC;
goto exit;
}
}

entry = &table->fds[index];
table->fds[index] = entry;

r = uv_mutex_init(&entry->mutex);
if (r != 0) {
Expand All @@ -258,7 +263,6 @@ static uvwasi_errno_t uvwasi__fd_table_insert(uvwasi_t* uvwasi,
entry->rights_base = rights_base;
entry->rights_inheriting = rights_inheriting;
entry->preopen = preopen;
entry->valid = 1;
table->used++;

if (wrap != NULL)
Expand Down Expand Up @@ -295,7 +299,7 @@ uvwasi_errno_t uvwasi_fd_table_init(uvwasi_t* uvwasi,
table->size = init_size;
table->fds = uvwasi__calloc(uvwasi,
init_size,
sizeof(struct uvwasi_fd_wrap_t));
sizeof(struct uvwasi_fd_wrap_t*));

if (table->fds == NULL) {
err = UVWASI_ENOMEM;
Expand Down Expand Up @@ -346,11 +350,11 @@ void uvwasi_fd_table_free(uvwasi_t* uvwasi, struct uvwasi_fd_table_t* table) {
return;

for (i = 0; i < table->size; i++) {
entry = &table->fds[i];
if (!entry->valid) continue;
entry = table->fds[i];
if (entry == NULL) continue;

uvwasi__free(uvwasi, entry->path);
uv_mutex_destroy(&entry->mutex);
uvwasi__free(uvwasi, entry);
}

uvwasi__free(uvwasi, table->fds);
Expand Down Expand Up @@ -455,9 +459,9 @@ uvwasi_errno_t uvwasi_fd_table_get(const struct uvwasi_fd_table_t* table,
goto exit;
}

entry = &table->fds[id];
entry = table->fds[id];

if (entry->valid != 1 || entry->id != id) {
if (entry == NULL || entry->id != id) {
err = UVWASI_EBADF;
goto exit;
}
Expand Down Expand Up @@ -494,16 +498,16 @@ uvwasi_errno_t uvwasi_fd_table_remove(uvwasi_t* uvwasi,
goto exit;
}

entry = &table->fds[id];
entry = table->fds[id];

if (entry->valid != 1 || entry->id != id) {
if (entry == NULL || entry->id != id) {
err = UVWASI_EBADF;
goto exit;
}

uvwasi__free(uvwasi, entry->path);
uv_mutex_destroy(&entry->mutex);
entry->valid = 0;
uvwasi__free(uvwasi, entry);
table->fds[id] = NULL;
table->used--;
err = UVWASI_ESUCCESS;
exit:
Expand Down

0 comments on commit 7579e29

Please sign in to comment.