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: RemoveAll doesn't remove nested un-readable directories #30555

Closed
cespare opened this issue Mar 4, 2019 · 5 comments
Closed

os: RemoveAll doesn't remove nested un-readable directories #30555

cespare opened this issue Mar 4, 2019 · 5 comments

Comments

@cespare
Copy link
Contributor

@cespare cespare commented Mar 4, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12 linux/amd64

Does this issue reproduce with the latest release?

Yes. It is a new regression in Go 1.12.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/caleb/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/caleb/p/go:/home/caleb"
GOPROXY=""
GORACE=""
GOROOT="/home/caleb/apps/go"
GOTMPDIR=""
GOTOOLDIR="/home/caleb/apps/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0
-fdebug-prefix-map=/tmp/go-build954814215=/tmp/go-build
-gno-record-gcc-switches"

What did you do?

os.RemoveAll doesn't remove nested directories if they are read-only. It does work if the top directory (i.e., the argument provided to RemoveAll) is read-only.

Demo:

package main

import (
	"log"
	"os"
	"path/filepath"
)

func main() {
	target := filepath.Join("d0", "d1")
	if err := os.MkdirAll(target, 0755); err != nil {
		log.Fatal(err)
	}
	if err := os.Chmod(target, 0300); err != nil {
		log.Fatal(err)
	}
	if err := os.RemoveAll("d0"); err != nil {
		log.Fatal(err)
	}
}

What did you expect to see?

No output; successful execution.

What did you see instead?

2019/03/03 18:36:49 openat d1: permission denied

Notes

This is a regression in Go 1.12. The bug is not present in Go 1.11.5.

The bug was introduced in https://golang.org/cl/146020, which was the fix for #27029.

Before that CL, RemoveAll would unconditionally attempt to call Remove on nested directories.

After that CL, it calls openat first and fails.

This regression is similar to (but distinct from) #29983, which was also caused by CL 146020.

cc @ianlancetaylor

@mvdan mvdan added the NeedsFix label Mar 4, 2019
@mvdan mvdan added this to the Go1.13 milestone Mar 4, 2019
@mvdan
Copy link
Member

@mvdan mvdan commented Mar 4, 2019

I presume this will need a backport into 1.12.1 as well.

/cc @ostenbom

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 4, 2019

Change https://golang.org/cl/165057 mentions this issue: os: fix RemoveAll can't remove unreadable directories

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 4, 2019

@gopherbot please open backport to 1.12

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 4, 2019

Backport issue(s) opened: #30579 (for 1.12).

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 gopherbot closed this in c746592 Mar 4, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 4, 2019

Change https://golang.org/cl/165058 mentions this issue: [release-branch.go1.12] os: remove unreadable directories in RemoveAll

gopherbot pushed a commit that referenced this issue Mar 5, 2019
Updates #30555
Fixes #30579

Change-Id: Ib894b4f3cdba23a18a69c9470cf69ceb83591a4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/165057
Run-TryBot: Baokun Lee <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit c746592)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165058
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.