Skip to content

make vmsplice unsafe#2777

Open
oech3 wants to merge 2 commits into
nix-rust:masterfrom
oech3:patch-1
Open

make vmsplice unsafe#2777
oech3 wants to merge 2 commits into
nix-rust:masterfrom
oech3:patch-1

Conversation

@oech3
Copy link
Copy Markdown

@oech3 oech3 commented Apr 16, 2026 •
edited
Loading

What does this PR do

make vmsplice unsafe. Closes #2633

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

@oech3 oech3 marked this pull request as draft April 16, 2026 17:19
@oech3 oech3 force-pushed the patch-1 branch 3 times, most recently from 19f9721 to eb540be Compare April 16, 2026 17:29
@xtqqczze

This comment was marked as resolved.

Sign in to view
@oech3

This comment was marked as resolved.

Sign in to view
@xtqqczze

This comment was marked as resolved.

Sign in to view
xtqqczze
xtqqczze reviewed Apr 17, 2026
View reviewed changes
Comment thread src/fcntl.rs Outdated
@oech3 oech3 force-pushed the patch-1 branch 2 times, most recently from e0b2a0e to 7aae780 Compare April 17, 2026 13:15
xtqqczze
xtqqczze reviewed Apr 17, 2026
View reviewed changes
Comment thread src/fcntl.rs Outdated
@oech3
Copy link
Copy Markdown
Author

oech3 commented Apr 17, 2026

I'd also describe this as "fixed" not "changed".

I'm hesitant to use "fixed" for breaking change.

xtqqczze
xtqqczze reviewed Apr 17, 2026
View reviewed changes
Comment thread changelog/2777.changed.md Outdated
@oech3 oech3 force-pushed the patch-1 branch 2 times, most recently from 10defca to b712ba2 Compare April 17, 2026 13:43
@oech3
Copy link
Copy Markdown
Author

oech3 commented Apr 17, 2026

Can we just remove this function?

@xtqqczze
Copy link
Copy Markdown
Contributor

xtqqczze commented Apr 29, 2026

is this ready for review?

@oech3
Copy link
Copy Markdown
Author

oech3 commented Apr 29, 2026

I still don't like current unsafe doc.

xtqqczze
xtqqczze reviewed Apr 29, 2026
View reviewed changes
Comment thread src/fcntl.rs Outdated
@oech3 oech3 marked this pull request as ready for review April 29, 2026 16:14
@oech3 oech3 requested a review from asomers April 29, 2026 16:14
asomers
asomers approved these changes Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

This is a definite improvement. But it's still way to unsafe for any but the bravest and most performance-obsessed programmer to use. It's beyond the scope of this PR, but I wonder if a safe Rust API could be devised? Something like make vmsplice only accept pointers to memory allocated by a special allocator, and only accept them by move, not by borrow.

@oech3
Copy link
Copy Markdown
Author

oech3 commented Apr 29, 2026

Someone said RAM allocated by mmap can is bit safer.

@oech3 oech3 mentioned this pull request Apr 29, 2026
Open
@oech3
Copy link
Copy Markdown
Author

oech3 commented Apr 29, 2026

I'm not familiar with kernel. But how about

input ->(sendfile etc...) memfd ->(mmap, edit and sendfile) -> output
?

oech3 and others added 2 commits May 11, 2026 16:16
@oech3 @SteveLauC
make vmsplice unsafe
60db6e7
@oech3 @xtqqczze @SteveLauC
Fix safety doc
13bfe13 
Co-authored-by: xtqqczze <45661989+xtqqczze@users.noreply.github.com>
@SteveLauC SteveLauC self-requested a review May 11, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asomers asomers asomers approved these changes

@SteveLauC SteveLauC Awaiting requested review from SteveLauC

+1 more reviewer

@xtqqczze xtqqczze xtqqczze left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

possible safety violation via vmsplice

3 participants

@oech3 @xtqqczze @asomers