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

File descriptor leak when spawning new process #26478

Closed
l0kod opened this issue Jun 21, 2015 · 6 comments
Closed

File descriptor leak when spawning new process #26478

l0kod opened this issue Jun 21, 2015 · 6 comments

Comments

@l0kod
Copy link
Contributor

l0kod commented Jun 21, 2015

When using an external library and/or FFI bindings, all non-cloexec file descriptors leak to all new processes spawned with Command::spawn().

This was introduced by the I/O reform because the getdtablesize() loop disappeared.

The getdtablesize() loop could be replaced with a walk_dir() through /proc/self/fd when available (cf. #12148 (comment)).

cc #12148
cc #22678
cc #24237
cc rust-lang/rfcs#941

cc @alexcrichton
cc @aturon

@alexcrichton
Copy link
Member

This is currently a very intentional part of the design of Command. There's a legitimate use case for having file descriptors inherited across forked processes on Unix (e.g. it's expected behavior), so going out of our way to prevent that from happening isn't in the same spirit as the rest of the I/O bindings of the standard library.

The standard library opens everything with CLOEXEC by default (and will one day provide the ability to not do so), but adding this sort of code on a post-fork would at most be a platform extension and would probably best be done with a "run this block post-fork" option to Command on unix. As such I believe this is an issue for the RFC repo instead of this one, so I'm going to close this issue.

@l0kod
Copy link
Contributor Author

l0kod commented Jun 22, 2015

This sounds like a security issue. A safe API should use an extra_io whitelist like did the previous I/O library.

@alexcrichton
Copy link
Member

@l0kod this is not a security issue for the standard library, if it's an issue at all it's an issue with the libraries that are opening file descriptors without CLOEXEC.

@l0kod
Copy link
Contributor Author

l0kod commented Jun 23, 2015

OK, so it's an issue with almost all bindings…

@l0kod
Copy link
Contributor Author

l0kod commented Jun 23, 2015

Is it OK if I create a PR to extend CommandExt for UNIX with something like close_fds_except(&mut, &[AsRawFd])?

@alexcrichton
Copy link
Member

We explicitly removed this behavior, so I would probably expect an RFC before a PR.

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

No branches or pull requests

2 participants