os: Add support for long path names on unix RemoveAll

On unix systems, long enough path names will fail when performing syscalls
like `Lstat`. The current RemoveAll uses several of these syscalls, and so
will fail for long paths. This can be risky, as it can let users "hide"
files from the system or otherwise make long enough paths for programs
to fail. By using `Unlinkat` and `Openat` syscalls instead, RemoveAll is
safer on unix systems. Initially implemented for linux, darwin, freebsd
and openbsd.

Fixes golang#27029

Co-authored-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
Co-authored-by: Julia Nedialkova <yulia.nedyalkova@sap.com>

src/internal/syscall/unix/link.go

@@ -0,0 +1,42 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build linux darwin freebsd openbsd netbsd dragonfly
+
+package unix
+
+import (
+	"syscall"
+	"unsafe"
+)
+
+func Unlinkat(dirfd int, path string, flags int) error {
+	var p *byte
+	p, err := syscall.BytePtrFromString(path)
+	if err != nil {
+		return err
+	}
+
+	_, _, errno := syscall.Syscall(unlinkatTrap, uintptr(dirfd), uintptr(unsafe.Pointer(p)), uintptr(flags))
+	if errno != 0 {
+		return errno
+	}
+
+	return nil
+}
+
+func Openat(dirfd int, path string, flags int, perm uint32) (int, error) {
+	var p *byte
+	p, err := syscall.BytePtrFromString(path)
+	if err != nil {
+		return 0, err
+	}
+
+	fd, _, errno := syscall.Syscall6(openatTrap, uintptr(dirfd), uintptr(unsafe.Pointer(p)), uintptr(flags), uintptr(perm), 0, 0)
+	if errno != 0 {
+		return 0, errno
+	}
+
+	return int(fd), nil
+}

src/internal/syscall/unix/link_sysnum_darwin.go

@@ -0,0 +1,10 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+const unlinkatTrap = uintptr(472)
+const openatTrap = uintptr(463)
+
+const AT_REMOVEDIR = 0x80

src/internal/syscall/unix/link_sysnum_dragonfly.go

@@ -0,0 +1,12 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
+const openatTrap uintptr = syscall.SYS_OPENAT
+
+const AT_REMOVEDIR = 0x2

src/internal/syscall/unix/link_sysnum_freebsd.go

@@ -0,0 +1,12 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
+const openatTrap uintptr = syscall.SYS_OPENAT
+
+const AT_REMOVEDIR = 0x800

src/internal/syscall/unix/link_sysnum_linux.go

@@ -0,0 +1,12 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
+const openatTrap uintptr = syscall.SYS_OPENAT
+
+const AT_REMOVEDIR = 0x200

src/internal/syscall/unix/link_sysnum_netbsd.go

@@ -0,0 +1,12 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
+const openatTrap uintptr = syscall.SYS_OPENAT
+
+const AT_REMOVEDIR = 0x800

src/internal/syscall/unix/link_sysnum_openbsd.go

@@ -0,0 +1,12 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
+const openatTrap uintptr = syscall.SYS_OPENAT
+
+const AT_REMOVEDIR = 0x08

src/os/path.go

@@ -5,7 +5,6 @@
 package os
 
 import (
-	"io"
 	"syscall"
 )
 
@@ -58,101 +57,3 @@ func MkdirAll(path string, perm FileMode) error {
 	}
 	return nil
 }
-
-// RemoveAll removes path and any children it contains.
-// It removes everything it can but returns the first error
-// it encounters. If the path does not exist, RemoveAll
-// returns nil (no error).
-func RemoveAll(path string) error {
-	// Simple case: if Remove works, we're done.
-	err := Remove(path)
-	if err == nil || IsNotExist(err) {
-		return nil
-	}
-
-	// Otherwise, is this a directory we need to recurse into?
-	dir, serr := Lstat(path)
-	if serr != nil {
-		if serr, ok := serr.(*PathError); ok && (IsNotExist(serr.Err) || serr.Err == syscall.ENOTDIR) {
-			return nil
-		}
-		return serr
-	}
-	if !dir.IsDir() {
-		// Not a directory; return the error from Remove.
-		return err
-	}
-
-	// Remove contents & return first error.
-	err = nil
-	for {
-		fd, err := Open(path)
-		if err != nil {
-			if IsNotExist(err) {
-				// Already deleted by someone else.
-				return nil
-			}
-			return err
-		}
-
-		const request = 1024
-		names, err1 := fd.Readdirnames(request)
-
-		// Removing files from the directory may have caused
-		// the OS to reshuffle it. Simply calling Readdirnames
-		// again may skip some entries. The only reliable way
-		// to avoid this is to close and re-open the
-		// directory. See issue 20841.
-		fd.Close()
-
-		for _, name := range names {
-			err1 := RemoveAll(path + string(PathSeparator) + name)
-			if err == nil {
-				err = err1
-			}
-		}
-
-		if err1 == io.EOF {
-			break
-		}
-		// If Readdirnames returned an error, use it.
-		if err == nil {
-			err = err1
-		}
-		if len(names) == 0 {
-			break
-		}
-
-		// We don't want to re-open unnecessarily, so if we
-		// got fewer than request names from Readdirnames, try
-		// simply removing the directory now. If that
-		// succeeds, we are done.
-		if len(names) < request {
-			err1 := Remove(path)
-			if err1 == nil || IsNotExist(err1) {
-				return nil
-			}
-
-			if err != nil {
-				// We got some error removing the
-				// directory contents, and since we
-				// read fewer names than we requested
-				// there probably aren't more files to
-				// remove. Don't loop around to read
-				// the directory again. We'll probably
-				// just get the same error.
-				return err
-			}
-		}
-	}
-
-	// Remove directory.
-	err1 := Remove(path)
-	if err1 == nil || IsNotExist(err1) {
-		return nil
-	}
-	if err == nil {
-		err = err1
-	}
-	return err
-}

src/os/path_test.go

@@ -5,7 +5,6 @@
 package os_test
 
 import (
-	"fmt"
 	"internal/testenv"
 	"io/ioutil"
 	. "os"
@@ -76,130 +75,6 @@ func TestMkdirAll(t *testing.T) {
 	}
 }
 
-func TestRemoveAll(t *testing.T) {
-	tmpDir := TempDir()
-	// Work directory.
-	path := tmpDir + "/_TestRemoveAll_"
-	fpath := path + "/file"
-	dpath := path + "/dir"
-
-	// Make directory with 1 file and remove.
-	if err := MkdirAll(path, 0777); err != nil {
-		t.Fatalf("MkdirAll %q: %s", path, err)
-	}
-	fd, err := Create(fpath)
-	if err != nil {
-		t.Fatalf("create %q: %s", fpath, err)
-	}
-	fd.Close()
-	if err = RemoveAll(path); err != nil {
-		t.Fatalf("RemoveAll %q (first): %s", path, err)
-	}
-	if _, err = Lstat(path); err == nil {
-		t.Fatalf("Lstat %q succeeded after RemoveAll (first)", path)
-	}
-
-	// Make directory with file and subdirectory and remove.
-	if err = MkdirAll(dpath, 0777); err != nil {
-		t.Fatalf("MkdirAll %q: %s", dpath, err)
-	}
-	fd, err = Create(fpath)
-	if err != nil {
-		t.Fatalf("create %q: %s", fpath, err)
-	}
-	fd.Close()
-	fd, err = Create(dpath + "/file")
-	if err != nil {
-		t.Fatalf("create %q: %s", fpath, err)
-	}
-	fd.Close()
-	if err = RemoveAll(path); err != nil {
-		t.Fatalf("RemoveAll %q (second): %s", path, err)
-	}
-	if _, err := Lstat(path); err == nil {
-		t.Fatalf("Lstat %q succeeded after RemoveAll (second)", path)
-	}
-
-	// Determine if we should run the following test.
-	testit := true
-	if runtime.GOOS == "windows" {
-		// Chmod is not supported under windows.
-		testit = false
-	} else {
-		// Test fails as root.
-		testit = Getuid() != 0
-	}
-	if testit {
-		// Make directory with file and subdirectory and trigger error.
-		if err = MkdirAll(dpath, 0777); err != nil {
-			t.Fatalf("MkdirAll %q: %s", dpath, err)
-		}
-
-		for _, s := range []string{fpath, dpath + "/file1", path + "/zzz"} {
-			fd, err = Create(s)
-			if err != nil {
-				t.Fatalf("create %q: %s", s, err)
-			}
-			fd.Close()
-		}
-		if err = Chmod(dpath, 0); err != nil {
-			t.Fatalf("Chmod %q 0: %s", dpath, err)
-		}
-
-		// No error checking here: either RemoveAll
-		// will or won't be able to remove dpath;
-		// either way we want to see if it removes fpath
-		// and path/zzz. Reasons why RemoveAll might
-		// succeed in removing dpath as well include:
-		//	* running as root
-		//	* running on a file system without permissions (FAT)
-		RemoveAll(path)
-		Chmod(dpath, 0777)
-
-		for _, s := range []string{fpath, path + "/zzz"} {
-			if _, err = Lstat(s); err == nil {
-				t.Fatalf("Lstat %q succeeded after partial RemoveAll", s)
-			}
-		}
-	}
-	if err = RemoveAll(path); err != nil {
-		t.Fatalf("RemoveAll %q after partial RemoveAll: %s", path, err)
-	}
-	if _, err = Lstat(path); err == nil {
-		t.Fatalf("Lstat %q succeeded after RemoveAll (final)", path)
-	}
-}
-
-// Test RemoveAll on a large directory.
-func TestRemoveAllLarge(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skipping in short mode")
-	}
-
-	tmpDir := TempDir()
-	// Work directory.
-	path := tmpDir + "/_TestRemoveAllLarge_"
-
-	// Make directory with 1000 files and remove.
-	if err := MkdirAll(path, 0777); err != nil {
-		t.Fatalf("MkdirAll %q: %s", path, err)
-	}
-	for i := 0; i < 1000; i++ {
-		fpath := fmt.Sprintf("%s/file%d", path, i)
-		fd, err := Create(fpath)
-		if err != nil {
-			t.Fatalf("create %q: %s", fpath, err)
-		}
-		fd.Close()
-	}
-	if err := RemoveAll(path); err != nil {
-		t.Fatalf("RemoveAll %q: %s", path, err)
-	}
-	if _, err := Lstat(path); err == nil {
-		t.Fatalf("Lstat %q succeeded after RemoveAll", path)
-	}
-}
-
 func TestMkdirAllWithSymlink(t *testing.T) {
 	testenv.MustHaveSymlink(t)