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

Standard file descriptor checks cause abort on startup with strict rlimit #96621

Closed
QuineDot opened this issue May 2, 2022 · 3 comments · Fixed by #96837
Closed

Standard file descriptor checks cause abort on startup with strict rlimit #96621

QuineDot opened this issue May 2, 2022 · 3 comments · Fixed by #96837
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. P-low Low priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@QuineDot
Copy link

QuineDot commented May 2, 2022

Cause

PR 75295 added polls to the standard file descriptors on startup; if they are closed, it attempts to reopen them to /dev/null. If RLIMIT_NOFILE is too low, the polling (or opening) will cause an abort.

Reported by @edre on URLO; diagnosed by @cuviper.

The rlimit could be checked ahead of time.

Code

I tried this Docker configuration

FROM rust:1.48
RUN rustup target add x86_64-unknown-linux-musl
RUN echo 'fn main(){println!("hi");}' > hi.rs; rustc --target x86_64-unknown-linux-musl -o hi hi.rs
RUN python3 -c 'import resource,subprocess;print(subprocess.run(["/hi"],preexec_fn=lambda: resource.setrlimit(resource.RLIMIT_NOFILE, (0,0))))'

I expected to see this happen: Successful execution

Instead, this happened: Process aborted

Version it worked on

It most recently worked on: Rust 1.47

Versions with regression

Rust 1.48 and forward.

Related issues

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

@QuineDot QuineDot added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 2, 2022
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-untriaged Untriaged performance or correctness regression. labels May 2, 2022
@the8472
Copy link
Member

the8472 commented May 2, 2022

Are file descriptors 0-2 open at the time of the abort?

Edit, ah, I see this is a limitation of poll(2), not of the process being incorrectly spawned. This could be solved by a fallback to calling fstat on individual fds if poll doesn't work.

@apiraino apiraino added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label May 4, 2022
@the8472
Copy link
Member

the8472 commented May 4, 2022

  EINVAL The  nfds  argument  is  greater than {OPEN_MAX}, or one of the fd members refers to a
         STREAM or multiplexer that is linked (directly or indirectly) downstream from a multi‐
         plexer.
   EINVAL The nfds value exceeds the RLIMIT_NOFILE value.

@m-ou-se m-ou-se added P-low Low priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 4, 2022
@m-ou-se
Copy link
Member

m-ou-se commented May 4, 2022

We're happy to review a patch for this. (E.g. falling back to fstat if poll returns EINVAL.)

@the8472 the8472 added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels May 4, 2022
@bors bors closed this as completed in ec55c61 Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. P-low Low priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants