From 9811ebe0c5634ce34fe42b3a755287b8486b0ebb Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 21 Jan 2020 23:08:38 -0500 Subject: [PATCH] deps: uvwasi: cherry-pick eea4508 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/cjihrig/uvwasi/issues/88 PR-URL: https://github.com/nodejs/node/pull/31432 Reviewed-By: Tobias Nießen Reviewed-By: Ben Noordhuis Reviewed-By: David Carlier Reviewed-By: Rich Trott Reviewed-By: Jiawen Geng --- deps/uvwasi/include/fd_table.h | 6 +++--- deps/uvwasi/src/fd_table.c | 26 ++++++++------------------ deps/uvwasi/src/uvwasi.c | 18 +++++++++++++----- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/deps/uvwasi/include/fd_table.h b/deps/uvwasi/include/fd_table.h index 9d88628e22c62d..fa8a44e7468d41 100644 --- a/deps/uvwasi/include/fd_table.h +++ b/deps/uvwasi/include/fd_table.h @@ -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, diff --git a/deps/uvwasi/src/fd_table.c b/deps/uvwasi/src/fd_table.c index c15ea09257506c..bc32f4dd28bbce 100644 --- a/deps/uvwasi/src/fd_table.c +++ b/deps/uvwasi/src/fd_table.c @@ -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; } diff --git a/deps/uvwasi/src/uvwasi.c b/deps/uvwasi/src/uvwasi.c index 9fa4db8a521111..53b7699f590e53 100644 --- a/deps/uvwasi/src/uvwasi.c +++ b/deps/uvwasi/src/uvwasi.c @@ -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; }