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

os: copy_file_range system call fails on some file systems #44272

Closed
ianlancetaylor opened this issue Feb 15, 2021 · 4 comments
Closed

os: copy_file_range system call fails on some file systems #44272

ianlancetaylor opened this issue Feb 15, 2021 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

On GNU/Linux systems we currently use the copy_file_range system call when copying from one open file to another via io.Copy or similar mechanisms. This seems to be exactly what copy_file_range is for. Unfortunately, on current Linux kernel versions this fails to work on certain file systems. The failure mode is that copy_file_range returns 0 even if the file actually contains data. A sample file system for which this occurs is the procfs file system that implements /proc.

For example, this program fails on Linux kernel version 5.7.17.

package main

import (
	"bytes"
	"fmt"
	"io"
	"io/ioutil"
	"os"
)

func main() {
	tmpfile, err := ioutil.TempFile("", "copy_file_range")
	if err != nil {
		fmt.Fprint(os.Stderr, err)
		os.Exit(1)
	}
	status := copy(tmpfile)
	os.Remove(tmpfile.Name())
	os.Exit(status)
}

func copy(tmpfile *os.File) int {
	cmdline, err := os.Open("/proc/self/cmdline")
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	defer cmdline.Close()
	if _, err := io.Copy(tmpfile, cmdline); err != nil {
		fmt.Fprintf(os.Stderr, "copy failed: %v\n", err)
		return 1
	}
	if err := tmpfile.Close(); err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	old, err := ioutil.ReadFile("/proc/self/cmdline")
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	new, err := ioutil.ReadFile(tmpfile.Name())
	if err != nil {
		fmt.Fprintln(os.Stderr, err)
		return 1
	}
	if !bytes.Equal(old, new) {
		fmt.Fprintf(os.Stderr, "got %q want %q\n", new, old)
		return 1
	}
	return 0
}

It seems likely, though not certain, that we can detect this case by treating a zero return from copy_file_range as indicating that copy_file_range is not supported for that file. In that case we can fall back to using read and write as usual. For the case of an empty file this will introduce additional system calls, but at least it will work correctly.

There is a mailing list discussion about this problem at https://lore.kernel.org/linux-fsdevel/20210126233840.GG4626@dread.disaster.area/T/#m05753578c7f7882f6e9ffe01f981bc223edef2b0.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 15, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.16 milestone Feb 15, 2021
@ianlancetaylor
Copy link
Contributor Author

@gopherbot Please open backport to Go 1.15.

This bug can cause file corruption when using io.Copy with files on certain file systems. There is no reasonable workaround other than avoiding io.Copy.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #44273 (for 1.15).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/291989 mentions this issue: internal/poll: if copy_file_range returns 0, assume it failed

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/292289 mentions this issue: [release-branch.go1.15] internal/poll: if copy_file_range returns 0, assume it failed

gopherbot pushed a commit that referenced this issue Feb 16, 2021
…assume it failed

On current Linux kernels copy_file_range does not correctly handle
files in certain special file systems, such as /proc. For those file
systems it fails to copy any data and returns zero. This breaks Go's
io.Copy for those files.

Fix the problem by assuming that if copy_file_range returns 0 the
first time it is called on a file, that that file is not supported.
In that case fall back to just using read. This will force an extra
system call when using io.Copy to copy a zero-sized normal file,
but at least it will work correctly.

For #36817
For #44272
Fixes #44273

Change-Id: I02e81872cb70fda0ce5485e2ea712f219132e614
Reviewed-on: https://go-review.googlesource.com/c/go/+/291989
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit 30641e3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/292289
@golang golang locked and limited conversation to collaborators Feb 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants