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

kernel_copy: avoid panic on unexpected OS error #91153

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

birkenfeld
Copy link
Contributor

According to documentation, the listed errnos should only occur
if the copy_file_range call cannot be made at all, so the
assert be correct. However, since in practice file system
drivers (incl. FUSE etc.) can return any errno they want, we
should not panic here.

Fixes #91152

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 23, 2021
@the8472
Copy link
Member

the8472 commented Nov 23, 2021

I think this might be an upstream bug or if FUSE is involved then a bug in that code and should be reported in the appropriate place instead of being papered over, no?

The assert simply surfaces something breaking the documented syscall API, so the bug lies there.

Edit: Nevermind, if we still bubble up the error we're not exactly papering over it. That may be good enough, even though not as loud as I'd like.

@birkenfeld
Copy link
Contributor Author

The question is how well-defined the syscall API really is. At least the manpage doesn't include wording like "if errno is X, Y or Z, no bytes can have been written", but only says "On error, copy_file_range() returns -1 and errno is set to indicate the error." 🤷‍♂️

According to documentation, the listed errnos should only occur
if the `copy_file_range` call cannot be made at all, so the
assert be correct.  However, since in practice file system
drivers (incl. FUSE etc.) can return any errno they want, we
should not panic here.

Fixes rust-lang#91152
@the8472
Copy link
Member

the8472 commented Nov 23, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Nov 23, 2021

📌 Commit b490ccc has been approved by the8472

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 23, 2021
@the8472 the8472 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Nov 23, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#90856 (Suggestion to wrap inner types using 'allocator_api' in tuple)
 - rust-lang#91103 (Inhibit clicks on summary's children)
 - rust-lang#91137 (Give people a single link they can click in the contributing guide)
 - rust-lang#91140 (Split inline const to two feature gates and mark expression position inline const complete)
 - rust-lang#91148 (Use `derive_default_enum` in the compiler)
 - rust-lang#91153 (kernel_copy: avoid panic on unexpected OS error)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3dc0011 into rust-lang:master Nov 23, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Copying multiple large files results in assertion failed: (left == right) in kernel_copy.rs:592
6 participants