diff options
Diffstat (limited to 'pkg/p9')
-rw-r--r-- | pkg/p9/file.go | 7 | ||||
-rw-r--r-- | pkg/p9/handlers.go | 221 | ||||
-rw-r--r-- | pkg/p9/local_server/local_server.go | 4 | ||||
-rw-r--r-- | pkg/p9/p9test/client_test.go | 71 | ||||
-rw-r--r-- | pkg/p9/p9test/mocks.go | 13 | ||||
-rw-r--r-- | pkg/p9/server.go | 3 |
6 files changed, 239 insertions, 80 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. |