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