Allow SetAttr and Allocate for deleted files

It's safe to call SetAttr and Allocate on fsgofer because the
file path is not used to open the file, if needed.

Fixes #3654

PiperOrigin-RevId: 407149393

5 files changed, 45 insertions, 8 deletions

diff --git a/pkg/p9/file.go b/pkg/p9/file.go
index 8d6af2d6b..b4b556cb9 100644
--- a/pkg/p9/file.go
+++ b/pkg/p9/file.go
@@ -21,13 +21,37 @@ import (
 	"gvisor.dev/gvisor/pkg/fd"
 )
 
+// AttacherOptions contains Attacher configuration.
+type AttacherOptions struct {
+	// SetAttrOnDeleted is set to true if it's safe to call File.SetAttr for
+	// deleted files.
+	SetAttrOnDeleted bool
+
+	// AllocateOnDeleted is set to true if it's safe to call File.Allocate for
+	// deleted files.
+	AllocateOnDeleted bool
+}
+
+// NoServerOptions partially implements Attacher with empty AttacherOptions.
+type NoServerOptions struct{}
+
+// ServerOptions implements Attacher.
+func (*NoServerOptions) ServerOptions() AttacherOptions {
+	return AttacherOptions{}
+}
+
 // Attacher is provided by the server.
 type Attacher interface {
 	// Attach returns a new File.
 	//
-	// The client-side attach will be translate to a series of walks from
+	// The client-side attach will be translated to a series of walks from
 	// the file returned by this Attach call.
 	Attach() (File, error)
+
+	// ServerOptions returns configuration options for this attach point.
+	//
+	// This is never caller in the client-side.
+	ServerOptions() AttacherOptions
 }
 
 // File is a set of operations corresponding to a single node.
@@ -301,7 +325,7 @@ type File interface {
 type DefaultWalkGetAttr struct{}
 
 // WalkGetAttr implements File.WalkGetAttr.
-func (DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, error) {
+func (*DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, error) {
 	return nil, nil, AttrMask{}, Attr{}, unix.ENOSYS
 }
 
@@ -309,7 +333,7 @@ func (DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, er
 type DisallowClientCalls struct{}
 
 // SetAttrClose implements File.SetAttrClose.
-func (DisallowClientCalls) SetAttrClose(SetAttrMask, SetAttr) error {
+func (*DisallowClientCalls) SetAttrClose(SetAttrMask, SetAttr) error {
 	panic("SetAttrClose should not be called on the server")
 }
 
@@ -321,6 +345,11 @@ func (*DisallowServerCalls) Renamed(File, string) {
 	panic("Renamed should not be called on the client")
 }
 
+// ServerOptions implements Attacher.
+func (*DisallowServerCalls) ServerOptions() AttacherOptions {
+	panic("ServerOptions should not be called on the client")
+}
+
 // DefaultMultiGetAttr implements File.MultiGetAttr() on top of File.
 func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 	stats := make([]FullStat, 0, len(names))
diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go
index 2657081e3..c85af5e9e 100644
--- a/pkg/p9/handlers.go
+++ b/pkg/p9/handlers.go
@@ -178,7 +178,7 @@ func (t *Tsetattrclunk) handle(cs *connState) message {
 		// This might be technically incorrect, as it's possible that
 		// there were multiple links and you can still change the
 		// corresponding inode information.
-		if ref.isDeleted() {
+		if !cs.server.options.SetAttrOnDeleted && ref.isDeleted() {
 			return unix.EINVAL
 		}
 
@@ -913,7 +913,7 @@ func (t *Tsetattr) handle(cs *connState) message {
 		// This might be technically incorrect, as it's possible that
 		// there were multiple links and you can still change the
 		// corresponding inode information.
-		if ref.isDeleted() {
+		if !cs.server.options.SetAttrOnDeleted && ref.isDeleted() {
 			return unix.EINVAL
 		}
 
@@ -946,7 +946,7 @@ func (t *Tallocate) handle(cs *connState) message {
 		}
 
 		// We don't allow allocate on files that have been deleted.
-		if ref.isDeleted() {
+		if !cs.server.options.AllocateOnDeleted && ref.isDeleted() {
 			return unix.EINVAL
 		}
 
diff --git a/pkg/p9/p9test/BUILD b/pkg/p9/p9test/BUILD
index 9c1ada0cb..f3eb8468b 100644
--- a/pkg/p9/p9test/BUILD
+++ b/pkg/p9/p9test/BUILD
@@ -12,7 +12,7 @@ MOCK_SRC_PACKAGE = "gvisor.dev/gvisor/pkg/p9"
 # mockgen_reflect is a source file that contains mock generation code that
 # imports the p9 package and generates a specification via reflection. The
 # usual generation path must be split into two distinct parts because the full
-# source tree is not available to all build targets. Only declared depencies
+# source tree is not available to all build targets. Only declared dependencies
 # are available (and even then, not the Go source files).
 genrule(
     name = "mockgen_reflect",
diff --git a/pkg/p9/p9test/p9test.go b/pkg/p9/p9test/p9test.go
index fd5ac3dbe..56939d100 100644
--- a/pkg/p9/p9test/p9test.go
+++ b/pkg/p9/p9test/p9test.go
@@ -307,6 +307,7 @@ func NewHarness(t *testing.T) (*Harness, *p9.Client) {
 	}
 
 	// Start the server, synchronized on exit.
+	h.Attacher.EXPECT().ServerOptions().Return(p9.AttacherOptions{}).Times(1)
 	server := p9.NewServer(h.Attacher)
 	h.wg.Add(1)
 	go func() {
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index 6428ad745..e7d129f9d 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -34,6 +34,8 @@ type Server struct {
 	// attacher provides the attach function.
 	attacher Attacher
 
+	options AttacherOptions
+
 	// pathTree is the full set of paths opened on this server.
 	//
 	// These may be across different connections, but rename operations
@@ -48,10 +50,15 @@ type Server struct {
 	renameMu sync.RWMutex
 }
 
-// NewServer returns a new server.
+// NewServer returns a new server. attacher may be nil.
 func NewServer(attacher Attacher) *Server {
+	opts := AttacherOptions{}
+	if attacher != nil {
+		opts = attacher.ServerOptions()
+	}
 	return &Server{
 		attacher: attacher,
+		options:  opts,
 		pathTree: newPathNode(),
 	}
 }