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

Don't use splice as general-purpose copy mechanism (data corruption reported by someone) #29044

Open
safinaskar opened this issue Sep 3, 2023 · 0 comments
Labels
bug 🐛 Programming errors, that need preferential fixing util-lib

Comments

@safinaskar
Copy link

safinaskar commented Sep 3, 2023

systemd version the issue has been seen with

62f643a (2023-09-03)

Used distribution

No response

Linux kernel version used

No response

CPU architectures issue was seen on

x86_64

Component

systemd

Description

Currently systemd uses splice in some places. In particular splice is used as one of possible general-purpose copy mechanisms in copy_bytes_full here:

n = splice(fdf, NULL, fdt, NULL, m, nonblock_pipe ? SPLICE_F_NONBLOCK : 0);
.

Please, stop this. splice is unreliable. Someone reported that his backups was silently corrupted because of splice: https://lore.kernel.org/lkml/c634a18e-9f2b-4746-bd8f-aa1d41e6ddf7@mattwhitlock.name/ . (The thread happened recently, in Jul 2023).

Linus said in the same thread: "I have grown to pretty much hate splice() over the years, just because it's been a constant source of sorrow in so many ways". ( https://lore.kernel.org/lkml/CAHk-=wgG_2cmHgZwKjydi7=iimyHyN8aessnbM9XQ9ufbaUz9g@mail.gmail.com/ ). Then he continued:

So I'd actually be more than happy to just say "let's decommission splice entirely, just keeping the interfaces alive for backwards compatibility"

Also Linus in that thread warned everybody who uses splice without understanding what it is: https://lore.kernel.org/lkml/CAHk-=wiq95bWiWLyz96ombPfpy=PNrc2KKyzJ2d+WMrxi6=OVA@mail.gmail.com/

So, please, don't use splice as general-purpose copy mechanism. Remove it from copy_bytes_full and possibly from other places.

Also, use of similar functions (sendfile and copy_file_range) in copy_bytes_full should be questioned, too. I. e. you should use them only if you are sure that they cannot cause silent data corruption. I spent some time reading sendfile source code and it seems that it is not very different from splice, so it seems that sendfile should be removed from copy_bytes_full, too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing util-lib
Development

No branches or pull requests

2 participants