summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/p9/file.go7
-rw-r--r--pkg/p9/handlers.go221
-rw-r--r--pkg/p9/local_server/local_server.go4
-rw-r--r--pkg/p9/p9test/client_test.go71
-rw-r--r--pkg/p9/p9test/mocks.go13
-rw-r--r--pkg/p9/server.go3
-rw-r--r--runsc/fsgofer/fsgofer.go23
-rw-r--r--runsc/fsgofer/fsgofer_test.go46
8 files changed, 257 insertions, 131 deletions
diff --git a/pkg/p9/file.go b/pkg/p9/file.go
index ae726f0b9..9723fa24d 100644
--- a/pkg/p9/file.go
+++ b/pkg/p9/file.go
@@ -20,10 +20,13 @@ import (
"gvisor.googlesource.com/gvisor/pkg/fd"
)
-// Attacher is provided by the user.
+// Attacher is provided by the server.
type Attacher interface {
// Attach returns a new File.
- Attach(attachName string) (File, error)
+ //
+ // The client-side attach will be translate to a series of walks from
+ // the file returned by this Attach call.
+ Attach() (File, error)
}
// File is a set of operations corresponding to a single node.
diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go
index 7da9eff5f..ea41f97c7 100644
--- a/pkg/p9/handlers.go
+++ b/pkg/p9/handlers.go
@@ -17,6 +17,7 @@ package p9
import (
"io"
"os"
+ "path"
"strings"
"sync/atomic"
"syscall"
@@ -94,39 +95,6 @@ func isSafeName(name string) bool {
}
// handle implements handler.handle.
-func (t *Twalk) handle(cs *connState) message {
- // Check the names.
- for _, name := range t.Names {
- if !isSafeName(name) {
- return newErr(syscall.EINVAL)
- }
- }
-
- // Lookup the FID.
- ref, ok := cs.LookupFID(t.FID)
- if !ok {
- return newErr(syscall.EBADF)
- }
- defer ref.DecRef()
-
- // Has it been opened already?
- if _, opened := ref.OpenFlags(); opened {
- return newErr(syscall.EBUSY)
- }
-
- // Do the walk.
- qids, sf, err := ref.file.Walk(t.Names)
- if err != nil {
- return newErr(err)
- }
-
- // Install the new FID.
- cs.InsertFID(t.NewFID, &fidRef{file: sf})
-
- return &Rwalk{QIDs: qids}
-}
-
-// handle implements handler.handle.
func (t *Tclunk) handle(cs *connState) message {
if !cs.DeleteFID(t.FID) {
return newErr(syscall.EBADF)
@@ -175,14 +143,57 @@ func (t *Tattach) handle(cs *connState) message {
return newErr(syscall.EINVAL)
}
- // Do the attach.
- sf, err := cs.server.attacher.Attach(t.Auth.AttachName)
+ // Must provide an absolute path.
+ if path.IsAbs(t.Auth.AttachName) {
+ // Trim off the leading / if the path is absolute. We always
+ // treat attach paths as absolute and call attach with the root
+ // argument on the server file for clarity.
+ t.Auth.AttachName = t.Auth.AttachName[1:]
+ }
+
+ // Do the attach on the root.
+ sf, err := cs.server.attacher.Attach()
if err != nil {
return newErr(err)
}
- cs.InsertFID(t.FID, &fidRef{file: sf})
+ _, valid, attr, err := sf.GetAttr(AttrMaskAll())
+ if err != nil {
+ sf.Close() // Drop file.
+ return newErr(err)
+ }
+ if !valid.Mode {
+ sf.Close() // Drop file.
+ return newErr(syscall.EINVAL)
+ }
+
+ // Build a transient reference.
+ root := &fidRef{
+ file: sf,
+ refs: 1,
+ walkable: attr.Mode.IsDir(),
+ }
+ defer root.DecRef()
+
+ // Attach the root?
+ if len(t.Auth.AttachName) == 0 {
+ cs.InsertFID(t.FID, root)
+ return &Rattach{}
+ }
+
+ // We want the same traversal checks to apply on attach, so always
+ // attach at the root and use the regular walk paths.
+ names := strings.Split(t.Auth.AttachName, "/")
+ _, target, _, attr, err := doWalk(cs, root, names)
+ if err != nil {
+ return newErr(err)
+ }
+
+ // Insert the FID.
+ cs.InsertFID(t.FID, &fidRef{
+ file: target,
+ walkable: attr.Mode.IsDir(),
+ })
- // Return an empty QID.
return &Rattach{}
}
@@ -678,15 +689,104 @@ func (t *Tflushf) handle(cs *connState) message {
return &Rflushf{}
}
-// handle implements handler.handle.
-func (t *Twalkgetattr) handle(cs *connState) message {
+// walkOne walks zero or one path elements.
+//
+// The slice passed as qids is append and returned.
+func walkOne(qids []QID, from File, names []string) ([]QID, File, AttrMask, Attr, error) {
+ if len(names) > 1 {
+ // We require exactly zero or one elements.
+ return nil, nil, AttrMask{}, Attr{}, syscall.EINVAL
+ }
+ var localQIDs []QID
+ localQIDs, sf, valid, attr, err := from.WalkGetAttr(names)
+ if err == syscall.ENOSYS {
+ localQIDs, sf, err = from.Walk(names)
+ if err != nil {
+ // No way to walk this element.
+ return nil, nil, AttrMask{}, Attr{}, err
+ }
+ // Need a manual getattr.
+ _, valid, attr, err = sf.GetAttr(AttrMaskAll())
+ if err != nil {
+ // Don't leak the file.
+ sf.Close()
+ }
+ }
+ if err != nil {
+ // Error walking, don't return anything.
+ return nil, nil, AttrMask{}, Attr{}, err
+ }
+ if len(localQIDs) != 1 {
+ // Expected a single QID.
+ sf.Close()
+ return nil, nil, AttrMask{}, Attr{}, syscall.EINVAL
+ }
+ return append(qids, localQIDs...), sf, valid, attr, nil
+}
+
+// doWalk walks from a given fidRef.
+//
+// This enforces that all intermediate nodes are walkable (directories).
+func doWalk(cs *connState, ref *fidRef, names []string) (qids []QID, sf File, valid AttrMask, attr Attr, err error) {
// Check the names.
- for _, name := range t.Names {
+ for _, name := range names {
if !isSafeName(name) {
- return newErr(syscall.EINVAL)
+ err = syscall.EINVAL
+ return
+ }
+ }
+
+ // Has it been opened already?
+ if _, opened := ref.OpenFlags(); opened {
+ err = syscall.EBUSY
+ return
+ }
+
+ // Is this an empty list? Handle specially. We don't actually need to
+ // validate anything since this is always permitted.
+ if len(names) == 0 {
+ return walkOne(nil, ref.file, nil)
+ }
+
+ // Is it walkable?
+ if !ref.walkable {
+ err = syscall.EINVAL
+ return
+ }
+
+ from := ref.file // Start at the passed ref.
+
+ // Do the walk, one element at a time.
+ for i := 0; i < len(names); i++ {
+ qids, sf, valid, attr, err = walkOne(qids, from, names[i:i+1])
+
+ // Close the intermediate file. Note that we don't close the
+ // first file because in that case we are walking from the
+ // existing reference.
+ if i > 0 {
+ from.Close()
+ }
+ from = sf // Use the new file.
+
+ // Was there an error walking?
+ if err != nil {
+ return nil, nil, AttrMask{}, Attr{}, err
+ }
+
+ // We won't allow beyond past symlinks; stop here if this isn't
+ // a proper directory and we have additional paths to walk.
+ if !valid.Mode || (!attr.Mode.IsDir() && i < len(names)-1) {
+ from.Close() // Not using the file object.
+ return nil, nil, AttrMask{}, Attr{}, syscall.EINVAL
}
}
+ // Success.
+ return qids, sf, valid, attr, nil
+}
+
+// handle implements handler.handle.
+func (t *Twalk) handle(cs *connState) message {
// Lookup the FID.
ref, ok := cs.LookupFID(t.FID)
if !ok {
@@ -694,26 +794,41 @@ func (t *Twalkgetattr) handle(cs *connState) message {
}
defer ref.DecRef()
- // Has it been opened already?
- if _, opened := ref.OpenFlags(); opened {
- return newErr(syscall.EBUSY)
+ // Do the walk.
+ qids, sf, _, attr, err := doWalk(cs, ref, t.Names)
+ if err != nil {
+ return newErr(err)
}
- // Do the walk.
- qids, sf, valid, attr, err := ref.file.WalkGetAttr(t.Names)
- if err == syscall.ENOSYS {
- qids, sf, err = ref.file.Walk(t.Names)
- if err != nil {
- return newErr(err)
- }
- _, valid, attr, err = sf.GetAttr(AttrMaskAll())
+ // Install the new FID.
+ cs.InsertFID(t.NewFID, &fidRef{
+ file: sf,
+ walkable: attr.Mode.IsDir(),
+ })
+
+ return &Rwalk{QIDs: qids}
+}
+
+// handle implements handler.handle.
+func (t *Twalkgetattr) handle(cs *connState) message {
+ // Lookup the FID.
+ ref, ok := cs.LookupFID(t.FID)
+ if !ok {
+ return newErr(syscall.EBADF)
}
+ defer ref.DecRef()
+
+ // Do the walk.
+ qids, sf, valid, attr, err := doWalk(cs, ref, t.Names)
if err != nil {
return newErr(err)
}
// Install the new FID.
- cs.InsertFID(t.NewFID, &fidRef{file: sf})
+ cs.InsertFID(t.NewFID, &fidRef{
+ file: sf,
+ walkable: attr.Mode.IsDir(),
+ })
return &Rwalkgetattr{QIDs: qids, Valid: valid, Attr: attr}
}
diff --git a/pkg/p9/local_server/local_server.go b/pkg/p9/local_server/local_server.go
index b4db44e27..cef3701a7 100644
--- a/pkg/p9/local_server/local_server.go
+++ b/pkg/p9/local_server/local_server.go
@@ -70,8 +70,8 @@ func (l *local) info() (p9.QID, os.FileInfo, error) {
}
// Attach implements p9.Attacher.Attach.
-func (l *local) Attach(name string) (p9.File, error) {
- return &local{path: path.Clean(name)}, nil
+func (l *local) Attach() (p9.File, error) {
+ return &local{path: "/"}, nil
}
// Walk implements p9.File.Walk.
diff --git a/pkg/p9/p9test/client_test.go b/pkg/p9/p9test/client_test.go
index 8e35d6017..34ddccd8b 100644
--- a/pkg/p9/p9test/client_test.go
+++ b/pkg/p9/p9test/client_test.go
@@ -50,8 +50,16 @@ func TestDonateFD(t *testing.T) {
// Craft attacher to attach to the mocked file which will return our
// temporary file.
- fileMock := &FileMock{OpenMock: OpenMock{File: f}}
- attacher := &AttachMock{File: fileMock}
+ fileMock := &FileMock{
+ OpenMock: OpenMock{File: f},
+ GetAttrMock: GetAttrMock{
+ // The mode must be valid always.
+ Valid: p9.AttrMask{Mode: true},
+ },
+ }
+ attacher := &AttachMock{
+ File: fileMock,
+ }
// Make socket pair.
serverSocket, clientSocket, err := unet.SocketPair(false)
@@ -139,15 +147,14 @@ func TestClient(t *testing.T) {
a.Called = false
a.File = sf
a.Err = nil
+ // The attached root must have a valid mode.
+ sf.GetAttrMock.Attr = p9.Attr{Mode: p9.ModeDirectory}
+ sf.GetAttrMock.Valid = p9.AttrMask{Mode: true}
var err error
- sfFile, err = c.Attach("foo")
+ sfFile, err = c.Attach("")
if !a.Called {
t.Errorf("Attach never Called?")
}
- if a.AttachName != "foo" {
- // This wasn't carried through?
- t.Errorf("attachName got %v wanted foo", a.AttachName)
- }
return err
},
},
@@ -155,6 +162,8 @@ func TestClient(t *testing.T) {
name: "bad-walk",
want: sentinelErr,
fn: func(c *p9.Client) error {
+ // Walk only called when WalkGetAttr not available.
+ sf.WalkGetAttrMock.Err = syscall.ENOSYS
sf.WalkMock.File = d
sf.WalkMock.Err = sentinelErr
_, _, err := sfFile.Walk([]string{"foo", "bar"})
@@ -164,21 +173,39 @@ func TestClient(t *testing.T) {
{
name: "walk-to-dir",
fn: func(c *p9.Client) error {
+ // Walk only called when WalkGetAttr not available.
+ sf.WalkGetAttrMock.Err = syscall.ENOSYS
sf.WalkMock.Called = false
+ sf.WalkMock.Names = nil
sf.WalkMock.File = d
sf.WalkMock.Err = nil
sf.WalkMock.QIDs = []p9.QID{{Type: 1}}
+ // All intermediate values must be directories.
+ d.WalkGetAttrMock.Err = syscall.ENOSYS
+ d.WalkMock.Called = false
+ d.WalkMock.Names = nil
+ d.WalkMock.File = d // Walk to self.
+ d.WalkMock.Err = nil
+ d.WalkMock.QIDs = []p9.QID{{Type: 1}}
+ d.GetAttrMock.Attr = p9.Attr{Mode: p9.ModeDirectory}
+ d.GetAttrMock.Valid = p9.AttrMask{Mode: true}
var qids []p9.QID
var err error
qids, _, err = sfFile.Walk([]string{"foo", "bar"})
if !sf.WalkMock.Called {
t.Errorf("Walk never Called?")
}
- if !reflect.DeepEqual(sf.WalkMock.Names, []string{"foo", "bar"}) {
- t.Errorf("got names %v wanted []{foo, bar}", sf.WalkMock.Names)
+ if !d.GetAttrMock.Called {
+ t.Errorf("GetAttr never Called?")
}
- if len(qids) != 1 || qids[0].Type != 1 {
- t.Errorf("got qids %v wanted []{{Type: 1}}", qids)
+ if !reflect.DeepEqual(sf.WalkMock.Names, []string{"foo"}) {
+ t.Errorf("got names %v wanted []{foo}", sf.WalkMock.Names)
+ }
+ if !reflect.DeepEqual(d.WalkMock.Names, []string{"bar"}) {
+ t.Errorf("got names %v wanted []{bar}", d.WalkMock.Names)
+ }
+ if len(qids) != 2 || qids[len(qids)-1].Type != 1 {
+ t.Errorf("got qids %v wanted []{..., {Type: 1}}", qids)
}
return err
},
@@ -187,11 +214,20 @@ func TestClient(t *testing.T) {
name: "walkgetattr-to-dir",
fn: func(c *p9.Client) error {
sf.WalkGetAttrMock.Called = false
+ sf.WalkGetAttrMock.Names = nil
sf.WalkGetAttrMock.File = d
sf.WalkGetAttrMock.Err = nil
sf.WalkGetAttrMock.QIDs = []p9.QID{{Type: 1}}
- sf.WalkGetAttrMock.Attr = p9.Attr{UID: 1}
+ sf.WalkGetAttrMock.Attr = p9.Attr{Mode: p9.ModeDirectory, UID: 1}
sf.WalkGetAttrMock.Valid = p9.AttrMask{Mode: true}
+ // See above.
+ d.WalkGetAttrMock.Called = false
+ d.WalkGetAttrMock.Names = nil
+ d.WalkGetAttrMock.File = d // Walk to self.
+ d.WalkGetAttrMock.Err = nil
+ d.WalkGetAttrMock.QIDs = []p9.QID{{Type: 1}}
+ d.WalkGetAttrMock.Attr = p9.Attr{Mode: p9.ModeDirectory, UID: 1}
+ d.WalkGetAttrMock.Valid = p9.AttrMask{Mode: true}
var qids []p9.QID
var err error
var mask p9.AttrMask
@@ -200,11 +236,14 @@ func TestClient(t *testing.T) {
if !sf.WalkGetAttrMock.Called {
t.Errorf("Walk never Called?")
}
- if !reflect.DeepEqual(sf.WalkGetAttrMock.Names, []string{"foo", "bar"}) {
- t.Errorf("got names %v wanted []{foo, bar}", sf.WalkGetAttrMock.Names)
+ if !reflect.DeepEqual(sf.WalkGetAttrMock.Names, []string{"foo"}) {
+ t.Errorf("got names %v wanted []{foo}", sf.WalkGetAttrMock.Names)
+ }
+ if !reflect.DeepEqual(d.WalkGetAttrMock.Names, []string{"bar"}) {
+ t.Errorf("got names %v wanted []{bar}", d.WalkGetAttrMock.Names)
}
- if len(qids) != 1 || qids[0].Type != 1 {
- t.Errorf("got qids %v wanted []{{Type: 1}}", qids)
+ if len(qids) != 2 || qids[len(qids)-1].Type != 1 {
+ t.Errorf("got qids %v wanted []{..., {Type: 1}}", qids)
}
if !reflect.DeepEqual(attr, sf.WalkGetAttrMock.Attr) {
t.Errorf("got attrs %s wanted %s", attr, sf.WalkGetAttrMock.Attr)
diff --git a/pkg/p9/p9test/mocks.go b/pkg/p9/p9test/mocks.go
index e10f206dd..9d039ac63 100644
--- a/pkg/p9/p9test/mocks.go
+++ b/pkg/p9/p9test/mocks.go
@@ -71,7 +71,8 @@ type WalkGetAttrMock struct {
// WalkGetAttr implements p9.File.WalkGetAttr.
func (w *WalkGetAttrMock) WalkGetAttr(names []string) ([]p9.QID, p9.File, p9.AttrMask, p9.Attr, error) {
- w.Called, w.Names = true, names
+ w.Called = true
+ w.Names = append(w.Names, names...)
return w.QIDs, w.File, w.Valid, w.Attr, w.Err
}
@@ -300,17 +301,14 @@ func (r *ReadlinkMock) Readlink() (string, error) {
type AttachMock struct {
Called bool
- // Args.
- AttachName string
-
// Return.
File p9.File
Err error
}
// Attach implements p9.Attacher.Attach.
-func (a *AttachMock) Attach(attachName string) (p9.File, error) {
- a.Called, a.AttachName = true, attachName
+func (a *AttachMock) Attach() (p9.File, error) {
+ a.Called = true
return a.File, a.Err
}
@@ -329,7 +327,8 @@ type WalkMock struct {
// Walk implements p9.File.Walk.
func (w *WalkMock) Walk(names []string) ([]p9.QID, p9.File, error) {
- w.Called, w.Names = true, names
+ w.Called = true
+ w.Names = append(w.Names, names...)
return w.QIDs, w.File, w.Err
}
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index 2965ae16e..28a273ac6 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -97,6 +97,9 @@ type fidRef struct {
// This is updated in handlers.go.
opened bool
+ // walkable indicates this fidRef may be walked.
+ walkable bool
+
// openFlags is the mode used in the open.
//
// This is updated in handlers.go.
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)
}
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-01 20:44:54 +0000