Add additional sanity checks for walk.

PiperOrigin-RevId: 212058684
Change-Id: I319709b9ffcfccb3231bac98df345d2a20eca24b

2 files changed, 18 insertions, 51 deletions

diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index b325afa63..9c4864cf1 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -117,17 +117,9 @@ func NewAttachPoint(prefix string, c Config) p9.Attacher {
 }
 
 // Attach implements p9.Attacher.
-func (a *attachPoint) Attach(appPath string) (p9.File, error) {
-	// Only proceed if 'appPath' is valid.
-	if !path.IsAbs(appPath) {
-		return nil, fmt.Errorf("invalid path %q", appPath)
-	}
-	if path.Clean(appPath) != appPath {
-		return nil, fmt.Errorf("invalid path %q", appPath)
-	}
-
-	root := path.Join(a.prefix, appPath)
-	fi, err := os.Stat(root)
+func (a *attachPoint) Attach() (p9.File, error) {
+	// Sanity check the prefix.
+	fi, err := os.Stat(a.prefix)
 	if err != nil {
 		return nil, err
 	}
@@ -136,14 +128,15 @@ func (a *attachPoint) Attach(appPath string) (p9.File, error) {
 		mode = os.O_RDONLY
 	}
 
-	f, err := os.OpenFile(root, mode|openFlags, 0)
+	// Open the root directory.
+	f, err := os.OpenFile(a.prefix, mode|openFlags, 0)
 	if err != nil {
-		return nil, fmt.Errorf("unable to open file %q, err: %v", root, err)
+		return nil, fmt.Errorf("unable to open file %q, err: %v", a.prefix, err)
 	}
 	stat, err := stat(int(f.Fd()))
 	if err != nil {
 		f.Close()
-		return nil, fmt.Errorf("failed to stat file %q, err: %v", root, err)
+		return nil, fmt.Errorf("failed to stat file %q, err: %v", a.prefix, err)
 	}
 
 	a.attachedMu.Lock()
@@ -154,7 +147,7 @@ func (a *attachPoint) Attach(appPath string) (p9.File, error) {
 	}
 	a.attached = true
 
-	return newLocalFile(a, f, root, stat)
+	return newLocalFile(a, f, a.prefix, stat)
 }
 
 // makeQID returns a unique QID for the given stat buffer.
diff --git a/runsc/fsgofer/fsgofer_test.go b/runsc/fsgofer/fsgofer_test.go
index fcece4e83..a500a2976 100644
--- a/runsc/fsgofer/fsgofer_test.go
+++ b/runsc/fsgofer/fsgofer_test.go
@@ -19,7 +19,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
-	"strings"
 	"syscall"
 	"testing"
 
@@ -88,9 +87,9 @@ func runCustom(t *testing.T, types []fileType, confs []Config, test func(*testin
 			defer os.RemoveAll(path)
 
 			a := NewAttachPoint(path, c)
-			root, err := a.Attach("/")
+			root, err := a.Attach()
 			if err != nil {
-				t.Fatalf("Attach(%q) failed, err: %v", "/", err)
+				t.Fatalf("Attach failed, err: %v", err)
 			}
 
 			_, file, err := root.Walk([]string{name})
@@ -115,9 +114,9 @@ func setup(ft fileType) (string, string, error) {
 
 	// First attach with writable configuration to setup tree.
 	a := NewAttachPoint(path, Config{})
-	root, err := a.Attach("/")
+	root, err := a.Attach()
 	if err != nil {
-		return "", "", fmt.Errorf("Attach(%q) failed, err: %v", "/", err)
+		return "", "", fmt.Errorf("Attach failed, err: %v", err)
 	}
 	defer root.Close()
 
@@ -618,9 +617,9 @@ func TestAttachFile(t *testing.T) {
 	}
 
 	a := NewAttachPoint(path, conf)
-	root, err := a.Attach("/")
+	root, err := a.Attach()
 	if err != nil {
-		t.Fatalf("Attach(%q) failed, err: %v", "/", err)
+		t.Fatalf("Attach failed, err: %v", err)
 	}
 
 	if _, _, _, err := root.Open(p9.ReadWrite); err != nil {
@@ -649,31 +648,6 @@ func TestAttachFile(t *testing.T) {
 	}
 }
 
-func TestAttachError(t *testing.T) {
-	conf := Config{ROMount: false}
-	root, err := ioutil.TempDir("", "root-")
-	if err != nil {
-		t.Fatalf("ioutil.TempDir() failed, err: %v", err)
-	}
-	defer os.RemoveAll(root)
-	a := NewAttachPoint(root, conf)
-
-	c := path.Join(root, "test")
-	if err := os.Mkdir(c, 0700); err != nil {
-		t.Fatalf("os.Create(%q) failed, err: %v", c, err)
-	}
-
-	for _, p := range []string{"test", "/test/../", "/test/./", "/test//"} {
-		_, err := a.Attach(p)
-		if err == nil {
-			t.Fatalf("Attach(%q) should have failed", p)
-		}
-		if want := "invalid path"; !strings.Contains(err.Error(), want) {
-			t.Fatalf("Attach(%q) wrong error, got: %v, wanted: %v", p, err, want)
-		}
-	}
-}
-
 func TestDoubleAttachError(t *testing.T) {
 	conf := Config{ROMount: false}
 	root, err := ioutil.TempDir("", "root-")
@@ -683,10 +657,10 @@ func TestDoubleAttachError(t *testing.T) {
 	defer os.RemoveAll(root)
 	a := NewAttachPoint(root, conf)
 
-	if _, err := a.Attach("/"); err != nil {
-		t.Fatalf("Attach(%q) failed: %v", "/", err)
+	if _, err := a.Attach(); err != nil {
+		t.Fatalf("Attach failed: %v", err)
 	}
-	if _, err := a.Attach("/"); err == nil {
-		t.Fatalf("Attach(%q) should have failed", "test")
+	if _, err := a.Attach(); err == nil {
+		t.Fatalf("Attach should have failed, got %v want non-nil", err)
 	}
 }