vfs: Remove vfs.DefaultDirectoryFD from embedding vfs.DefaultFD.

This fixes the implementation ambiguity issues when a filesystem
implementation embeds vfs.DefaultDirectoryFD to its directory FD along
with an internal common fileDescription utility.

For similar reasons also removes FileDescriptionDefaultImpl from
DynamicBytesFileDescriptionImpl.

PiperOrigin-RevId: 263795513

5 files changed, 22 insertions, 34 deletions

diff --git a/pkg/sentry/fsimpl/ext/file_description.go b/pkg/sentry/fsimpl/ext/file_description.go
index d244cf1e7..a0065343b 100644
--- a/pkg/sentry/fsimpl/ext/file_description.go
+++ b/pkg/sentry/fsimpl/ext/file_description.go
@@ -16,18 +16,16 @@ package ext
 
 import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
-	"gvisor.dev/gvisor/pkg/sentry/arch"
 	"gvisor.dev/gvisor/pkg/sentry/context"
-	"gvisor.dev/gvisor/pkg/sentry/usermem"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 	"gvisor.dev/gvisor/pkg/syserror"
-	"gvisor.dev/gvisor/pkg/waiter"
 )
 
 // fileDescription is embedded by ext implementations of
 // vfs.FileDescriptionImpl.
 type fileDescription struct {
 	vfsfd vfs.FileDescription
+	vfs.FileDescriptionDefaultImpl
 
 	// flags is the same as vfs.OpenOptions.Flags which are passed to
 	// vfs.FilesystemImpl.OpenAt.
@@ -82,29 +80,7 @@ func (fd *fileDescription) StatFS(ctx context.Context) (linux.Statfs, error) {
 	return stat, nil
 }
 
-// Readiness implements waiter.Waitable.Readiness analogously to
-// file_operations::poll == NULL in Linux.
-func (fd *fileDescription) Readiness(mask waiter.EventMask) waiter.EventMask {
-	// include/linux/poll.h:vfs_poll() => DEFAULT_POLLMASK
-	return waiter.EventIn | waiter.EventOut
-}
-
-// EventRegister implements waiter.Waitable.EventRegister analogously to
-// file_operations::poll == NULL in Linux.
-func (fd *fileDescription) EventRegister(e *waiter.Entry, mask waiter.EventMask) {}
-
-// EventUnregister implements waiter.Waitable.EventUnregister analogously to
-// file_operations::poll == NULL in Linux.
-func (fd *fileDescription) EventUnregister(e *waiter.Entry) {}
-
 // Sync implements vfs.FileDescriptionImpl.Sync.
 func (fd *fileDescription) Sync(ctx context.Context) error {
 	return nil
 }
-
-// Ioctl implements vfs.FileDescriptionImpl.Ioctl.
-func (fd *fileDescription) Ioctl(ctx context.Context, uio usermem.IO, args arch.SyscallArguments) (uintptr, error) {
-	// ioctl(2) specifies that ENOTTY must be returned if the file descriptor is
-	// not associated with a character special device (which is unimplemented).
-	return 0, syserror.ENOTTY
-}
diff --git a/pkg/sentry/fsimpl/memfs/memfs.go b/pkg/sentry/fsimpl/memfs/memfs.go
index 59612da14..45cd42b3e 100644
--- a/pkg/sentry/fsimpl/memfs/memfs.go
+++ b/pkg/sentry/fsimpl/memfs/memfs.go
@@ -258,6 +258,7 @@ func (i *inode) direntType() uint8 {
 // vfs.FileDescriptionImpl.
 type fileDescription struct {
 	vfsfd vfs.FileDescription
+	vfs.FileDescriptionDefaultImpl
 
 	flags uint32 // status flags; immutable
 }
diff --git a/pkg/sentry/fsimpl/memfs/regular_file.go b/pkg/sentry/fsimpl/memfs/regular_file.go
index 7a16d5719..55f869798 100644
--- a/pkg/sentry/fsimpl/memfs/regular_file.go
+++ b/pkg/sentry/fsimpl/memfs/regular_file.go
@@ -46,7 +46,6 @@ func (fs *filesystem) newRegularFile(creds *auth.Credentials, mode uint16) *inod
 
 type regularFileFD struct {
 	fileDescription
-	vfs.FileDescriptionDefaultImpl
 
 	// These are immutable.
 	readable bool
diff --git a/pkg/sentry/vfs/file_description_impl_util.go b/pkg/sentry/vfs/file_description_impl_util.go
index 4d1d447ed..ba230da72 100644
--- a/pkg/sentry/vfs/file_description_impl_util.go
+++ b/pkg/sentry/vfs/file_description_impl_util.go
@@ -28,6 +28,16 @@ import (
 	"gvisor.dev/gvisor/pkg/waiter"
 )
 
+// The following design pattern is strongly recommended for filesystem
+// implementations to adapt:
+//   - Have a local fileDescription struct (containing FileDescription) which
+//     embeds FileDescriptionDefaultImpl and overrides the default methods
+//     which are common to all fd implementations for that for that filesystem
+//     like StatusFlags, SetStatusFlags, Stat, SetStat, StatFS, etc.
+//   - This should be embedded in all file description implementations as the
+//     first field by value.
+//   - Directory FDs would also embed DirectoryFileDescriptionDefaultImpl.
+
 // FileDescriptionDefaultImpl may be embedded by implementations of
 // FileDescriptionImpl to obtain implementations of many FileDescriptionImpl
 // methods with default behavior analogous to Linux's.
@@ -119,11 +129,8 @@ func (FileDescriptionDefaultImpl) Ioctl(ctx context.Context, uio usermem.IO, arg
 
 // DirectoryFileDescriptionDefaultImpl may be embedded by implementations of
 // FileDescriptionImpl that always represent directories to obtain
-// implementations of non-directory I/O methods that return EISDIR, and
-// implementations of other methods consistent with FileDescriptionDefaultImpl.
-type DirectoryFileDescriptionDefaultImpl struct {
-	FileDescriptionDefaultImpl
-}
+// implementations of non-directory I/O methods that return EISDIR.
+type DirectoryFileDescriptionDefaultImpl struct{}
 
 // PRead implements FileDescriptionImpl.PRead.
 func (DirectoryFileDescriptionDefaultImpl) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts ReadOptions) (int64, error) {
@@ -153,8 +160,6 @@ func (DirectoryFileDescriptionDefaultImpl) Write(ctx context.Context, src userme
 // DynamicBytesFileDescriptionImpl.SetDataSource() must be called before first
 // use.
 type DynamicBytesFileDescriptionImpl struct {
-	FileDescriptionDefaultImpl
-
 	data     DynamicBytesSource // immutable
 	mu       sync.Mutex         // protects the following fields
 	buf      bytes.Buffer
diff --git a/pkg/sentry/vfs/file_description_impl_util_test.go b/pkg/sentry/vfs/file_description_impl_util_test.go
index cef7e3ce5..511b829fc 100644
--- a/pkg/sentry/vfs/file_description_impl_util_test.go
+++ b/pkg/sentry/vfs/file_description_impl_util_test.go
@@ -29,11 +29,18 @@ import (
 	"gvisor.dev/gvisor/pkg/syserror"
 )
 
+// fileDescription is the common fd struct which a filesystem implementation
+// embeds in all of its file description implementations as required.
+type fileDescription struct {
+	vfsfd FileDescription
+	FileDescriptionDefaultImpl
+}
+
 // genCountFD is a read-only FileDescriptionImpl representing a regular file
 // that contains the number of times its DynamicBytesSource.Generate()
 // implementation has been called.
 type genCountFD struct {
-	vfsfd FileDescription
+	fileDescription
 	DynamicBytesFileDescriptionImpl
 
 	count uint64 // accessed using atomic memory ops