Skip to content

Commit

Permalink
deps: uvwasi: cherry-pick eea4508
Browse files Browse the repository at this point in the history
Original commit message:

    prevent race conditions with uvwasi_fd_close()

    uvwasi_fd_close() performed the following operations:

    - lock the file descriptor mutex
    - close the file
    - release the file descriptor mutex
    - call the file table's remove() function

    Once the fd's mutex is released, another thread could
    acquire it before the fd is removed from the file
    table. If this happens, remove() could destroy a held
    mutex.

    This commit updates uvwasi_fd_close() to perform the
    entire sequence while holding the file table's lock,
    preventing new acquisitions of the fd's mutex.

    Fixes: nodejs/uvwasi#88

PR-URL: #31432
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
  • Loading branch information
cjihrig authored and codebytere committed Feb 17, 2020
1 parent 2fe0ed3 commit 9811ebe
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
6 changes: 3 additions & 3 deletions deps/uvwasi/include/fd_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ uvwasi_errno_t uvwasi_fd_table_get_nolock(struct uvwasi_fd_table_t* table,
struct uvwasi_fd_wrap_t** wrap,
uvwasi_rights_t rights_base,
uvwasi_rights_t rights_inheriting);
uvwasi_errno_t uvwasi_fd_table_remove(struct uvwasi_s* uvwasi,
struct uvwasi_fd_table_t* table,
const uvwasi_fd_t id);
uvwasi_errno_t uvwasi_fd_table_remove_nolock(struct uvwasi_s* uvwasi,
struct uvwasi_fd_table_t* table,
const uvwasi_fd_t id);
uvwasi_errno_t uvwasi_fd_table_renumber(struct uvwasi_s* uvwasi,
struct uvwasi_fd_table_t* table,
const uvwasi_fd_t dst,
Expand Down
26 changes: 8 additions & 18 deletions deps/uvwasi/src/fd_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,37 +306,27 @@ uvwasi_errno_t uvwasi_fd_table_get_nolock(struct uvwasi_fd_table_t* table,
}


uvwasi_errno_t uvwasi_fd_table_remove(uvwasi_t* uvwasi,
struct uvwasi_fd_table_t* table,
const uvwasi_fd_t id) {
uvwasi_errno_t uvwasi_fd_table_remove_nolock(uvwasi_t* uvwasi,
struct uvwasi_fd_table_t* table,
const uvwasi_fd_t id) {
struct uvwasi_fd_wrap_t* entry;
uvwasi_errno_t err;

if (table == NULL)
return UVWASI_EINVAL;

uv_rwlock_wrlock(&table->rwlock);

if (id >= table->size) {
err = UVWASI_EBADF;
goto exit;
}
if (id >= table->size)
return UVWASI_EBADF;

entry = table->fds[id];

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

uv_mutex_destroy(&entry->mutex);
uvwasi__free(uvwasi, entry);
table->fds[id] = NULL;
table->used--;
err = UVWASI_ESUCCESS;
exit:
uv_rwlock_wrunlock(&table->rwlock);
return err;
return UVWASI_ESUCCESS;
}


Expand Down
18 changes: 13 additions & 5 deletions deps/uvwasi/src/uvwasi.c
Original file line number Diff line number Diff line change
Expand Up @@ -878,18 +878,26 @@ uvwasi_errno_t uvwasi_fd_close(uvwasi_t* uvwasi, uvwasi_fd_t fd) {
if (uvwasi == NULL)
return UVWASI_EINVAL;

err = uvwasi_fd_table_get(&uvwasi->fds, fd, &wrap, 0, 0);
uvwasi_fd_table_lock(&uvwasi->fds);

err = uvwasi_fd_table_get_nolock(&uvwasi->fds, fd, &wrap, 0, 0);
if (err != UVWASI_ESUCCESS)
return err;
goto exit;

r = uv_fs_close(NULL, &req, wrap->fd, NULL);
uv_mutex_unlock(&wrap->mutex);
uv_fs_req_cleanup(&req);

if (r != 0)
return uvwasi__translate_uv_error(r);
if (r != 0) {
err = uvwasi__translate_uv_error(r);
goto exit;
}

err = uvwasi_fd_table_remove_nolock(uvwasi, &uvwasi->fds, fd);

return uvwasi_fd_table_remove(uvwasi, &uvwasi->fds, fd);
exit:
uvwasi_fd_table_unlock(&uvwasi->fds);
return err;
}


Expand Down

0 comments on commit 9811ebe

Please sign in to comment.