On Fri, Feb 12, 2021 at 8:28 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Feb 12, 2021 at 07:59:04AM -0800, Ian Lance Taylor wrote:
> > On Fri, Feb 12, 2021 at 7:45 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Fri, Feb 12, 2021 at 07:33:57AM -0800, Ian Lance Taylor wrote:
> > > > On Fri, Feb 12, 2021 at 12:38 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > Why are people trying to use copy_file_range on simple /proc and /sys
> > > > > files in the first place? They can not seek (well most can not), so
> > > > > that feels like a "oh look, a new syscall, let's use it everywhere!"
> > > > > problem that userspace should not do.
> > > >
> > > > This may have been covered elsewhere, but it's not that people are
> > > > saying "let's use copy_file_range on files in /proc." It's that the
> > > > Go language standard library provides an interface to operating system
> > > > files. When Go code uses the standard library function io.Copy to
> > > > copy the contents of one open file to another open file, then on Linux
> > > > kernels 5.3 and greater the Go standard library will use the
> > > > copy_file_range system call. That seems to be exactly what
> > > > copy_file_range is intended for. Unfortunately it appears that when
> > > > people writing Go code open a file in /proc and use io.Copy the
> > > > contents to another open file, copy_file_range does nothing and
> > > > reports success. There isn't anything on the copy_file_range man page
> > > > explaining this limitation, and there isn't any documented way to know
> > > > that the Go standard library should not use copy_file_range on certain
> > > > files.
> > >
> > > But, is this a bug in the kernel in that the syscall being made is not
> > > working properly, or a bug in that Go decided to do this for all types
> > > of files not knowing that some types of files can not handle this?
> > >
> > > If the kernel has always worked this way, I would say that Go is doing
> > > the wrong thing here. If the kernel used to work properly, and then
> > > changed, then it's a regression on the kernel side.
> > >
> > > So which is it?
> >
> > I don't work on the kernel, so I can't tell you which it is. You will
> > have to decide.
>
> As you have the userspace code, it should be easier for you to test this
> on an older kernel. I don't have your userspace code...
Sorry, I'm not sure what you are asking.
I've attached a sample Go program. On kernel version 2.6.32 this
program exits 0. On kernel version 5.7.17 it prints
got "" want "./foo\x00"
and exits with status 1.
This program hardcodes the string "/proc/self/cmdline" for
convenience, but of course the same results would happen if this were
a generic copy program that somebody invoked with /proc/self/cmdline
as a command line option.
I could write the same program in C easily enough, by explicitly
calling copy_file_range. Would it help to see a sample C program?
> > From my perspective, as a kernel user rather than a kernel developer,
> > a system call that silently fails for certain files and that provides
> > no way to determine either 1) ahead of time that the system call will
> > fail, or 2) after the call that the system call did fail, is a useless
> > system call.
>
> Great, then don't use copy_file_range() yet as it seems like it fits
> that category at the moment :)
That seems like an unfortunate result, but if that is the determining
opinion then I guess that is what we will have to do in the Go
standard library.
Ian
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
}
