Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Race condition in uvwasi_fd_close() #88

Closed
cjihrig opened this issue Jan 21, 2020 · 0 comments · Fixed by #91
Closed

Race condition in uvwasi_fd_close() #88

cjihrig opened this issue Jan 21, 2020 · 0 comments · Fixed by #91

Comments

@cjihrig
Copy link
Collaborator

cjihrig commented Jan 21, 2020

uvwasi_fd_close() locks the fd mutex, closes the file, releases the mutex, then calls uvwasi_fd_table_remove() to free the fd table entry and destroy the entry's mutex.

Another thread could acquire the mutex once it is unlocked, and uvwasi_fd_table_remove() would destroy it.
cjihrig added a commit that referenced this issue Jan 22, 2020
@cjihrig
prevent race conditions with uvwasi_fd_close()
cdc8280 
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: #88
@cjihrig cjihrig mentioned this issue Jan 22, 2020
Merged
cjihrig added a commit that referenced this issue Jan 22, 2020
@cjihrig
prevent race conditions with uvwasi_fd_close()
61f0660 
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: #88
cjihrig added a commit that referenced this issue Jan 22, 2020
@cjihrig
prevent race conditions with uvwasi_fd_close()
eea4508 
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: #88
cjihrig added a commit to cjihrig/node that referenced this issue Jan 23, 2020
@cjihrig
deps: uvwasi: cherry-pick eea4508
2fe7459 
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
Trott pushed a commit to Trott/io.js that referenced this issue Jan 23, 2020
@cjihrig @Trott
deps: uvwasi: cherry-pick eea4508
30173c6 
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: nodejs#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>
codebytere pushed a commit to nodejs/node that referenced this issue Feb 17, 2020
@cjihrig @codebytere
deps: uvwasi: cherry-pick eea4508
9811ebe 
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>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 15, 2020
@cjihrig @codebytere
deps: uvwasi: cherry-pick eea4508
4ccd7dd 
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>
codebytere pushed a commit to nodejs/node that referenced this issue Mar 17, 2020
@cjihrig @codebytere
deps: uvwasi: cherry-pick eea4508
3fb3079 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

prevent race conditions with uvwasi_fd_close() nodejs/uvwasi
1 participant
@cjihrig