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.

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

Unsoundness in FdSet #1572

Closed
taylordotfish opened this issue Oct 17, 2021 · 3 comments · Fixed by #1575
Closed

Unsoundness in FdSet #1572

taylordotfish opened this issue Oct 17, 2021 · 3 comments · Fixed by #1575
Milestone

Comments

@taylordotfish
Copy link
Contributor

FdSet::insert, FdSet::remove, and FdSet::contains call FD_SET(), FD_CLR(), and FD_ISSET() without any bounds checking, but those underlying libc functions cause undefined behavior when provided a file descriptor outside of the range 0..FD_SETSIZE.

@taylordotfish taylordotfish changed the title Unsoundness in FdSet::insert and FdSet::remove Unsoundness in FdSet Oct 17, 2021
@asomers
Copy link
Member

asomers commented Oct 18, 2021

Yep, it's true. Would you be willing to submit a PR to fix it?

@taylordotfish
Copy link
Contributor Author

Sure. Is adding bounds checking an acceptable approach, despite the minor runtime cost?

@asomers
Copy link
Member

asomers commented Oct 18, 2021

Yes. After all, select users aren't overly concerned about the runtime cost.

@asomers asomers added this to the 0.23.1 milestone Oct 18, 2021
taylordotfish added a commit to taylordotfish/nix that referenced this issue Oct 19, 2021
Ensure file descriptors are nonnegative and less than `FD_SETSIZE`.
(Fixes nix-rust#1572.)
taylordotfish added a commit to taylordotfish/nix that referenced this issue Oct 21, 2021
Ensure file descriptors are nonnegative and less than `FD_SETSIZE`.
(Fixes nix-rust#1572.)
bors bot added a commit that referenced this issue Oct 22, 2021
1575: Fix unsoundness in FdSet methods r=asomers a=taylordotfish

Ensure file descriptors are nonnegative and less than `FD_SETSIZE`. (Fixes #1572.)

Co-authored-by: taylor.fish <[email protected]>
@bors bors bot closed this as completed in dc80b4c Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants