Move VFS2 handling of FD readability/writability to vfs.FileDescription.

PiperOrigin-RevId: 291006713
This commit is contained in:
Jamie Liu 2020-01-22 12:27:16 -08:00 committed by gVisor bot
parent 159992300d
commit 5ab1213a6c
10 changed files with 111 additions and 54 deletions

View File

@ -157,7 +157,9 @@ func (in *inode) open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, flags uint32) (*v
switch in.impl.(type) {
case *regularFile:
var fd regularFileFD
fd.vfsfd.Init(&fd, flags, mnt, vfsd, &vfs.FileDescriptionOptions{})
if err := fd.vfsfd.Init(&fd, flags, mnt, vfsd, &vfs.FileDescriptionOptions{}); err != nil {
return nil, err
}
return &fd.vfsfd, nil
case *directory:
// Can't open directories writably. This check is not necessary for a read
@ -166,7 +168,9 @@ func (in *inode) open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, flags uint32) (*v
return nil, syserror.EISDIR
}
var fd directoryFD
fd.vfsfd.Init(&fd, flags, mnt, vfsd, &vfs.FileDescriptionOptions{})
if err := fd.vfsfd.Init(&fd, flags, mnt, vfsd, &vfs.FileDescriptionOptions{}); err != nil {
return nil, err
}
return &fd.vfsfd, nil
case *symlink:
if flags&linux.O_PATH == 0 {

View File

@ -55,7 +55,9 @@ func (f *DynamicBytesFile) Init(creds *auth.Credentials, ino uint64, data vfs.Dy
// Open implements Inode.Open.
func (f *DynamicBytesFile) Open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, flags uint32) (*vfs.FileDescription, error) {
fd := &DynamicBytesFD{}
fd.Init(rp.Mount(), vfsd, f.data, flags)
if err := fd.Init(rp.Mount(), vfsd, f.data, flags); err != nil {
return nil, err
}
return &fd.vfsfd, nil
}
@ -80,10 +82,13 @@ type DynamicBytesFD struct {
}
// Init initializes a DynamicBytesFD.
func (fd *DynamicBytesFD) Init(m *vfs.Mount, d *vfs.Dentry, data vfs.DynamicBytesSource, flags uint32) {
func (fd *DynamicBytesFD) Init(m *vfs.Mount, d *vfs.Dentry, data vfs.DynamicBytesSource, flags uint32) error {
if err := fd.vfsfd.Init(fd, flags, m, d, &vfs.FileDescriptionOptions{}); err != nil {
return err
}
fd.inode = d.Impl().(*Dentry).inode
fd.SetDataSource(data)
fd.vfsfd.Init(fd, flags, m, d, &vfs.FileDescriptionOptions{})
return nil
}
// Seek implements vfs.FileDescriptionImpl.Seek.

View File

@ -43,9 +43,16 @@ type GenericDirectoryFD struct {
}
// Init initializes a GenericDirectoryFD.
func (fd *GenericDirectoryFD) Init(m *vfs.Mount, d *vfs.Dentry, children *OrderedChildren, flags uint32) {
func (fd *GenericDirectoryFD) Init(m *vfs.Mount, d *vfs.Dentry, children *OrderedChildren, flags uint32) error {
if vfs.AccessTypesForOpenFlags(flags)&vfs.MayWrite != 0 {
// Can't open directories for writing.
return syserror.EISDIR
}
if err := fd.vfsfd.Init(fd, flags, m, d, &vfs.FileDescriptionOptions{}); err != nil {
return err
}
fd.children = children
fd.vfsfd.Init(fd, flags, m, d, &vfs.FileDescriptionOptions{})
return nil
}
// VFSFileDescription returns a pointer to the vfs.FileDescription representing

View File

@ -115,7 +115,9 @@ func (fs *filesystem) newReadonlyDir(creds *auth.Credentials, mode linux.FileMod
func (d *readonlyDir) Open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, flags uint32) (*vfs.FileDescription, error) {
fd := &kernfs.GenericDirectoryFD{}
fd.Init(rp.Mount(), vfsd, &d.OrderedChildren, flags)
if err := fd.Init(rp.Mount(), vfsd, &d.OrderedChildren, flags); err != nil {
return nil, err
}
return fd.VFSFileDescription(), nil
}
@ -225,7 +227,9 @@ func TestReadStaticFile(t *testing.T) {
defer sys.Destroy()
pop := sys.PathOpAtRoot("file1")
fd, err := sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{})
fd, err := sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{
Flags: linux.O_RDONLY,
})
if err != nil {
t.Fatalf("OpenAt for PathOperation %+v failed: %v", pop, err)
}
@ -258,7 +262,9 @@ func TestCreateNewFileInStaticDir(t *testing.T) {
// Close the file. The file should persist.
fd.DecRef()
fd, err = sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{})
fd, err = sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{
Flags: linux.O_RDONLY,
})
if err != nil {
t.Fatalf("OpenAt(pop:%+v) = %+v failed: %v", pop, fd, err)
}
@ -272,7 +278,9 @@ func TestDirFDReadWrite(t *testing.T) {
defer sys.Destroy()
pop := sys.PathOpAtRoot("/")
fd, err := sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{})
fd, err := sys.VFS.OpenAt(sys.Ctx, sys.Creds, &pop, &vfs.OpenOptions{
Flags: linux.O_RDONLY,
})
if err != nil {
t.Fatalf("OpenAt for PathOperation %+v failed: %v", pop, err)
}
@ -282,7 +290,7 @@ func TestDirFDReadWrite(t *testing.T) {
if _, err := fd.Read(sys.Ctx, usermem.BytesIOSequence([]byte{}), vfs.ReadOptions{}); err != syserror.EISDIR {
t.Fatalf("Read for directory FD failed with unexpected error: %v", err)
}
if _, err := fd.Write(sys.Ctx, usermem.BytesIOSequence([]byte{}), vfs.WriteOptions{}); err != syserror.EISDIR {
if _, err := fd.Write(sys.Ctx, usermem.BytesIOSequence([]byte{}), vfs.WriteOptions{}); err != syserror.EBADF {
t.Fatalf("Write for directory FD failed with unexpected error: %v", err)
}
}

View File

@ -337,19 +337,12 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, flags uint32,
return nil, err
}
}
mnt := rp.Mount()
switch impl := d.inode.impl.(type) {
case *regularFile:
var fd regularFileFD
fd.readable = vfs.MayReadFileWithOpenFlags(flags)
fd.writable = vfs.MayWriteFileWithOpenFlags(flags)
if fd.writable {
if err := mnt.CheckBeginWrite(); err != nil {
return nil, err
}
// mnt.EndWrite() is called by regularFileFD.Release().
if err := fd.vfsfd.Init(&fd, flags, rp.Mount(), &d.vfsd, &vfs.FileDescriptionOptions{}); err != nil {
return nil, err
}
fd.vfsfd.Init(&fd, flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{})
if flags&linux.O_TRUNC != 0 {
impl.mu.Lock()
impl.data.Truncate(0, impl.memFile)
@ -363,7 +356,9 @@ func (d *dentry) open(ctx context.Context, rp *vfs.ResolvingPath, flags uint32,
return nil, syserror.EISDIR
}
var fd directoryFD
fd.vfsfd.Init(&fd, flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{})
if err := fd.vfsfd.Init(&fd, flags, rp.Mount(), &d.vfsd, &vfs.FileDescriptionOptions{}); err != nil {
return nil, err
}
return &fd.vfsfd, nil
case *symlink:
// Can't open symlinks without O_PATH (which is unimplemented).

View File

@ -50,11 +50,10 @@ type namedPipeFD struct {
func newNamedPipeFD(ctx context.Context, np *namedPipe, rp *vfs.ResolvingPath, vfsd *vfs.Dentry, flags uint32) (*vfs.FileDescription, error) {
var err error
var fd namedPipeFD
fd.VFSPipeFD, err = np.pipe.NewVFSPipeFD(ctx, rp, vfsd, &fd.vfsfd, flags)
fd.VFSPipeFD, err = np.pipe.NewVFSPipeFD(ctx, vfsd, &fd.vfsfd, flags)
if err != nil {
return nil, err
}
mnt := rp.Mount()
fd.vfsfd.Init(&fd, flags, mnt, vfsd, &vfs.FileDescriptionOptions{})
fd.vfsfd.Init(&fd, flags, rp.Mount(), vfsd, &vfs.FileDescriptionOptions{})
return &fd.vfsfd, nil
}

View File

@ -101,10 +101,6 @@ func (rf *regularFile) truncate(size uint64) (bool, error) {
type regularFileFD struct {
fileDescription
// These are immutable.
readable bool
writable bool
// off is the file offset. off is accessed using atomic memory operations.
// offMu serializes operations that may mutate off.
off int64
@ -113,16 +109,11 @@ type regularFileFD struct {
// Release implements vfs.FileDescriptionImpl.Release.
func (fd *regularFileFD) Release() {
if fd.writable {
fd.vfsfd.VirtualDentry().Mount().EndWrite()
}
// noop
}
// PRead implements vfs.FileDescriptionImpl.PRead.
func (fd *regularFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) {
if !fd.readable {
return 0, syserror.EINVAL
}
if offset < 0 {
return 0, syserror.EINVAL
}
@ -147,9 +138,6 @@ func (fd *regularFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts
// PWrite implements vfs.FileDescriptionImpl.PWrite.
func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) {
if !fd.writable {
return 0, syserror.EINVAL
}
if offset < 0 {
return 0, syserror.EINVAL
}

View File

@ -66,7 +66,7 @@ func NewVFSPipe(sizeBytes, atomicIOBytes int64) *VFSPipe {
// for read and write will succeed both in blocking and nonblocking mode. POSIX
// leaves this behavior undefined. This can be used to open a FIFO for writing
// while there are no readers available." - fifo(7)
func (vp *VFSPipe) NewVFSPipeFD(ctx context.Context, rp *vfs.ResolvingPath, vfsd *vfs.Dentry, vfsfd *vfs.FileDescription, flags uint32) (*VFSPipeFD, error) {
func (vp *VFSPipe) NewVFSPipeFD(ctx context.Context, vfsd *vfs.Dentry, vfsfd *vfs.FileDescription, flags uint32) (*VFSPipeFD, error) {
vp.mu.Lock()
defer vp.mu.Unlock()
@ -76,7 +76,7 @@ func (vp *VFSPipe) NewVFSPipeFD(ctx context.Context, rp *vfs.ResolvingPath, vfsd
return nil, syserror.EINVAL
}
vfd, err := vp.open(rp, vfsd, vfsfd, flags)
vfd, err := vp.open(vfsd, vfsfd, flags)
if err != nil {
return nil, err
}
@ -118,19 +118,13 @@ func (vp *VFSPipe) NewVFSPipeFD(ctx context.Context, rp *vfs.ResolvingPath, vfsd
}
// Preconditions: vp.mu must be held.
func (vp *VFSPipe) open(rp *vfs.ResolvingPath, vfsd *vfs.Dentry, vfsfd *vfs.FileDescription, flags uint32) (*VFSPipeFD, error) {
func (vp *VFSPipe) open(vfsd *vfs.Dentry, vfsfd *vfs.FileDescription, flags uint32) (*VFSPipeFD, error) {
var fd VFSPipeFD
fd.flags = flags
fd.readable = vfs.MayReadFileWithOpenFlags(flags)
fd.writable = vfs.MayWriteFileWithOpenFlags(flags)
fd.vfsfd = vfsfd
fd.pipe = &vp.pipe
if fd.writable {
// The corresponding Mount.EndWrite() is in VFSPipe.Release().
if err := rp.Mount().CheckBeginWrite(); err != nil {
return nil, err
}
}
switch {
case fd.readable && fd.writable:

View File

@ -49,8 +49,23 @@ type FileDescription struct {
// A reference is held on vd. vd is immutable.
vd VirtualDentry
// opts contains options passed to FileDescription.Init(). opts is
// immutable.
opts FileDescriptionOptions
// readable is MayReadFileWithOpenFlags(statusFlags). readable is
// immutable.
//
// readable is analogous to Linux's FMODE_READ.
readable bool
// writable is MayWriteFileWithOpenFlags(statusFlags). If writable is true,
// the FileDescription holds a write count on vd.mount. writable is
// immutable.
//
// writable is analogous to Linux's FMODE_WRITE.
writable bool
// impl is the FileDescriptionImpl associated with this Filesystem. impl is
// immutable. This should be the last field in FileDescription.
impl FileDescriptionImpl
@ -77,10 +92,17 @@ type FileDescriptionOptions struct {
UseDentryMetadata bool
}
// Init must be called before first use of fd. It takes references on mnt and
// d. statusFlags is the initial file description status flags, which is
// usually the full set of flags passed to open(2).
func (fd *FileDescription) Init(impl FileDescriptionImpl, statusFlags uint32, mnt *Mount, d *Dentry, opts *FileDescriptionOptions) {
// Init must be called before first use of fd. If it succeeds, it takes
// references on mnt and d. statusFlags is the initial file description status
// flags, which is usually the full set of flags passed to open(2).
func (fd *FileDescription) Init(impl FileDescriptionImpl, statusFlags uint32, mnt *Mount, d *Dentry, opts *FileDescriptionOptions) error {
writable := MayWriteFileWithOpenFlags(statusFlags)
if writable {
if err := mnt.CheckBeginWrite(); err != nil {
return err
}
}
fd.refs = 1
fd.statusFlags = statusFlags | linux.O_LARGEFILE
fd.vd = VirtualDentry{
@ -89,7 +111,10 @@ func (fd *FileDescription) Init(impl FileDescriptionImpl, statusFlags uint32, mn
}
fd.vd.IncRef()
fd.opts = *opts
fd.readable = MayReadFileWithOpenFlags(statusFlags)
fd.writable = writable
fd.impl = impl
return nil
}
// IncRef increments fd's reference count.
@ -117,6 +142,9 @@ func (fd *FileDescription) TryIncRef() bool {
func (fd *FileDescription) DecRef() {
if refs := atomic.AddInt64(&fd.refs, -1); refs == 0 {
fd.impl.Release()
if fd.writable {
fd.vd.mount.EndWrite()
}
fd.vd.DecRef()
} else if refs < 0 {
panic("FileDescription.DecRef() called without holding a reference")
@ -194,6 +222,16 @@ func (fd *FileDescription) SetStatusFlags(ctx context.Context, creds *auth.Crede
return nil
}
// IsReadable returns true if fd was opened for reading.
func (fd *FileDescription) IsReadable() bool {
return fd.readable
}
// IsWritable returns true if fd was opened for writing.
func (fd *FileDescription) IsWritable() bool {
return fd.writable
}
// Impl returns the FileDescriptionImpl associated with fd.
func (fd *FileDescription) Impl() FileDescriptionImpl {
return fd.impl
@ -241,6 +279,8 @@ type FileDescriptionImpl interface {
// Errors:
//
// - If opts.Flags specifies unsupported options, PRead returns EOPNOTSUPP.
//
// Preconditions: The FileDescription was opened for reading.
PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts ReadOptions) (int64, error)
// Read is similar to PRead, but does not specify an offset.
@ -254,6 +294,8 @@ type FileDescriptionImpl interface {
// Errors:
//
// - If opts.Flags specifies unsupported options, Read returns EOPNOTSUPP.
//
// Preconditions: The FileDescription was opened for reading.
Read(ctx context.Context, dst usermem.IOSequence, opts ReadOptions) (int64, error)
// PWrite writes src to the file, starting at the given offset, and returns
@ -268,6 +310,8 @@ type FileDescriptionImpl interface {
//
// - If opts.Flags specifies unsupported options, PWrite returns
// EOPNOTSUPP.
//
// Preconditions: The FileDescription was opened for writing.
PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts WriteOptions) (int64, error)
// Write is similar to PWrite, but does not specify an offset, which is
@ -281,6 +325,8 @@ type FileDescriptionImpl interface {
// Errors:
//
// - If opts.Flags specifies unsupported options, Write returns EOPNOTSUPP.
//
// Preconditions: The FileDescription was opened for writing.
Write(ctx context.Context, src usermem.IOSequence, opts WriteOptions) (int64, error)
// IterDirents invokes cb on each entry in the directory represented by the
@ -411,11 +457,17 @@ func (fd *FileDescription) StatFS(ctx context.Context) (linux.Statfs, error) {
// offset, and returns the number of bytes read. PRead is permitted to return
// partial reads with a nil error.
func (fd *FileDescription) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts ReadOptions) (int64, error) {
if !fd.readable {
return 0, syserror.EBADF
}
return fd.impl.PRead(ctx, dst, offset, opts)
}
// Read is similar to PRead, but does not specify an offset.
func (fd *FileDescription) Read(ctx context.Context, dst usermem.IOSequence, opts ReadOptions) (int64, error) {
if !fd.readable {
return 0, syserror.EBADF
}
return fd.impl.Read(ctx, dst, opts)
}
@ -423,11 +475,17 @@ func (fd *FileDescription) Read(ctx context.Context, dst usermem.IOSequence, opt
// offset, and returns the number of bytes written. PWrite is permitted to
// return partial writes with a nil error.
func (fd *FileDescription) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts WriteOptions) (int64, error) {
if !fd.writable {
return 0, syserror.EBADF
}
return fd.impl.PWrite(ctx, src, offset, opts)
}
// Write is similar to PWrite, but does not specify an offset.
func (fd *FileDescription) Write(ctx context.Context, src usermem.IOSequence, opts WriteOptions) (int64, error) {
if !fd.writable {
return 0, syserror.EBADF
}
return fd.impl.Write(ctx, src, opts)
}

View File

@ -94,14 +94,13 @@ func GenericCheckPermissions(creds *auth.Credentials, ats AccessTypes, isDir boo
// the set of accesses permitted for the opened file:
//
// - O_TRUNC causes MayWrite to be set in the returned AccessTypes (since it
// mutates the file), but does not permit the opened to write to the file
// mutates the file), but does not permit writing to the open file description
// thereafter.
//
// - "Linux reserves the special, nonstandard access mode 3 (binary 11) in
// flags to mean: check for read and write permission on the file and return a
// file descriptor that can't be used for reading or writing." - open(2). Thus
// AccessTypesForOpenFlags returns MayRead|MayWrite in this case, but
// filesystems are responsible for ensuring that access is denied.
// AccessTypesForOpenFlags returns MayRead|MayWrite in this case.
//
// Use May{Read,Write}FileWithOpenFlags() for these checks instead.
func AccessTypesForOpenFlags(flags uint32) AccessTypes {