Skip to content

Fix fcntl(F_DUPFD) to make it comply with min_fd#740

Draft
sangho2 wants to merge 2 commits intomainfrom
sanghle/fix-dupfd
Draft

Fix fcntl(F_DUPFD) to make it comply with min_fd#740
sangho2 wants to merge 2 commits intomainfrom
sanghle/fix-dupfd

Conversation

@sangho2
Copy link
Copy Markdown
Contributor

@sangho2 sangho2 commented Apr 2, 2026

This PR fixes a bug of the current fcntl(F_DUPFD) implementation. fcntl(F_DUPFD) should return an fd greater than or equal to the arg (min_fd) but it doesn't.

@sangho2 sangho2 force-pushed the sanghle/fix-dupfd branch from aa3a2e8 to 6dfae55 Compare April 2, 2026 17:08
@sangho2 sangho2 changed the title Fix F_DUPFD minimum descriptor allocation Fix fcntl(F_DUPFD) to make it comply with min_fd Apr 2, 2026
@sangho2 sangho2 marked this pull request as ready for review April 2, 2026 20:46
}
DupFdRequest::LowestAvailable => Ok(rds.fd_into_raw_integer(fd)),
DupFdRequest::LowestAtOrAbove(min_fd) => {
Ok(rds.fd_into_raw_integer_at_or_above(fd, min_fd))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weidong has a similar fix but do not require adding a new interface to the LiteBox:

} else if let Some(min_fd) = min_fd {
#[allow(clippy::maybe_infinite_iter)]
let raw_fd = (min_fd..)
.find(|&raw_fd| !rds.is_alive(raw_fd))
.expect("raw fd search should always find a slot");
let success = rds.fd_into_specific_raw_integer(fd, raw_fd);
assert!(success);
Ok(raw_fd)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did have a similar fix but there is a potential TOCTOU race.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is that? rds is locked at the top.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found that my old one and Weidong's one are similar but different. Revised this PR.
Draft for now because this PR is not urgent.

@sangho2 sangho2 marked this pull request as draft April 3, 2026 14:10
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 3, 2026

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

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 this pull request may close these issues.

2 participants