diff --git a/pkg/sentry/fs/ashmem/area.go b/pkg/sentry/fs/ashmem/area.go index e1971f91c..3b8d6ca89 100644 --- a/pkg/sentry/fs/ashmem/area.go +++ b/pkg/sentry/fs/ashmem/area.go @@ -139,7 +139,7 @@ func (a *Area) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.MM } // Ioctl implements fs.FileOperations.Ioctl. -func (a *Area) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (a *Area) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // Switch on ioctl request. switch args[1].Uint() { case linux.AshmemSetNameIoctl: diff --git a/pkg/sentry/fs/binder/binder.go b/pkg/sentry/fs/binder/binder.go index acd5d7164..eef54d787 100644 --- a/pkg/sentry/fs/binder/binder.go +++ b/pkg/sentry/fs/binder/binder.go @@ -162,7 +162,7 @@ func (bp *Proc) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.M // Ioctl implements fs.FileOperations.Ioctl. // // TODO(b/30946773): Implement. -func (bp *Proc) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (bp *Proc) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // Switch on ioctl request. switch uint32(args[1].Int()) { case linux.BinderVersionIoctl: diff --git a/pkg/sentry/fs/file_operations.go b/pkg/sentry/fs/file_operations.go index 2c5bd1b19..d86f5bf45 100644 --- a/pkg/sentry/fs/file_operations.go +++ b/pkg/sentry/fs/file_operations.go @@ -155,5 +155,16 @@ type FileOperations interface { // refer. // // Preconditions: The AddressSpace (if any) that io refers to is activated. - Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) + Ioctl(ctx context.Context, file *File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) +} + +// FifoSizer is an interface for setting and getting the size of a pipe. +type FifoSizer interface { + // FifoSize returns the pipe capacity in bytes. + FifoSize(ctx context.Context, file *File) (int64, error) + + // SetFifoSize sets the new pipe capacity in bytes. + // + // The new size is returned (which may be capped). + SetFifoSize(size int64) (int64, error) } diff --git a/pkg/sentry/fs/file_overlay.go b/pkg/sentry/fs/file_overlay.go index 29e5618f4..c6cbd5631 100644 --- a/pkg/sentry/fs/file_overlay.go +++ b/pkg/sentry/fs/file_overlay.go @@ -397,9 +397,49 @@ func (f *overlayFileOperations) UnstableAttr(ctx context.Context, file *File) (U return f.lower.UnstableAttr(ctx) } -// Ioctl implements fs.FileOperations.Ioctl and always returns ENOTTY. -func (*overlayFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { - return 0, syserror.ENOTTY +// Ioctl implements fs.FileOperations.Ioctl. +func (f *overlayFileOperations) Ioctl(ctx context.Context, overlayFile *File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { + f.upperMu.Lock() + defer f.upperMu.Unlock() + + if f.upper == nil { + // It's possible that ioctl changes the file. Since we don't know all + // possible ioctls, only allow them to propagate to the upper. Triggering a + // copy up on any ioctl would be too drastic. In the future, it can have a + // list of ioctls that are safe to send to lower and a list that triggers a + // copy up. + return 0, syserror.ENOTTY + } + return f.upper.FileOperations.Ioctl(ctx, f.upper, io, args) +} + +// FifoSize implements FifoSizer.FifoSize. +func (f *overlayFileOperations) FifoSize(ctx context.Context, overlayFile *File) (rv int64, err error) { + err = f.onTop(ctx, overlayFile, func(file *File, ops FileOperations) error { + sz, ok := ops.(FifoSizer) + if !ok { + return syserror.EINVAL + } + rv, err = sz.FifoSize(ctx, file) + return err + }) + return +} + +// SetFifoSize implements FifoSizer.SetFifoSize. +func (f *overlayFileOperations) SetFifoSize(size int64) (rv int64, err error) { + f.upperMu.Lock() + defer f.upperMu.Unlock() + + if f.upper == nil { + // Named pipes cannot be copied up and changes to the lower are prohibited. + return 0, syserror.EINVAL + } + sz, ok := f.upper.FileOperations.(FifoSizer) + if !ok { + return 0, syserror.EINVAL + } + return sz.SetFifoSize(size) } // readdirEntries returns a sorted map of directory entries from the diff --git a/pkg/sentry/fs/fsutil/file.go b/pkg/sentry/fs/fsutil/file.go index d7ddc038b..626b9126a 100644 --- a/pkg/sentry/fs/fsutil/file.go +++ b/pkg/sentry/fs/fsutil/file.go @@ -219,7 +219,7 @@ func GenericConfigureMMap(file *fs.File, m memmap.Mappable, opts *memmap.MMapOpt type FileNoIoctl struct{} // Ioctl implements fs.FileOperations.Ioctl. -func (FileNoIoctl) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (FileNoIoctl) Ioctl(context.Context, *fs.File, usermem.IO, arch.SyscallArguments) (uintptr, error) { return 0, syserror.ENOTTY } diff --git a/pkg/sentry/fs/host/tty.go b/pkg/sentry/fs/host/tty.go index 0ceedb200..2526412a4 100644 --- a/pkg/sentry/fs/host/tty.go +++ b/pkg/sentry/fs/host/tty.go @@ -114,7 +114,7 @@ func (t *TTYFileOperations) Release() { } // Ioctl implements fs.FileOperations.Ioctl. -func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (t *TTYFileOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // Ignore arg[0]. This is the real FD: fd := t.fileOperations.iops.fileState.FD() ioctl := args[1].Uint64() diff --git a/pkg/sentry/fs/inotify.go b/pkg/sentry/fs/inotify.go index 41238f3fa..c7f4e2d13 100644 --- a/pkg/sentry/fs/inotify.go +++ b/pkg/sentry/fs/inotify.go @@ -202,7 +202,7 @@ func (i *Inotify) UnstableAttr(ctx context.Context, file *File) (UnstableAttr, e } // Ioctl implements fs.FileOperations.Ioctl. -func (i *Inotify) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (i *Inotify) Ioctl(ctx context.Context, _ *File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { switch args[1].Int() { case linux.FIONREAD: i.evMu.Lock() diff --git a/pkg/sentry/fs/tty/master.go b/pkg/sentry/fs/tty/master.go index 54dd3d6b7..92ec1ca18 100644 --- a/pkg/sentry/fs/tty/master.go +++ b/pkg/sentry/fs/tty/master.go @@ -144,7 +144,7 @@ func (mf *masterFileOperations) Write(ctx context.Context, _ *fs.File, src userm } // Ioctl implements fs.FileOperations.Ioctl. -func (mf *masterFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (mf *masterFileOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { switch cmd := args[1].Uint(); cmd { case linux.FIONREAD: // linux.FIONREAD == linux.TIOCINQ // Get the number of bytes in the output queue read buffer. diff --git a/pkg/sentry/fs/tty/slave.go b/pkg/sentry/fs/tty/slave.go index 2d68978dd..e30266404 100644 --- a/pkg/sentry/fs/tty/slave.go +++ b/pkg/sentry/fs/tty/slave.go @@ -128,7 +128,7 @@ func (sf *slaveFileOperations) Write(ctx context.Context, _ *fs.File, src userme } // Ioctl implements fs.FileOperations.Ioctl. -func (sf *slaveFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (sf *slaveFileOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { switch cmd := args[1].Uint(); cmd { case linux.FIONREAD: // linux.FIONREAD == linux.TIOCINQ // Get the number of bytes in the input queue read buffer. diff --git a/pkg/sentry/kernel/pipe/pipe.go b/pkg/sentry/kernel/pipe/pipe.go index 8e49070a9..247e2928e 100644 --- a/pkg/sentry/kernel/pipe/pipe.go +++ b/pkg/sentry/kernel/pipe/pipe.go @@ -39,19 +39,6 @@ const ( MaximumPipeSize = 8 << 20 ) -// Sizer is an interface for setting and getting the size of a pipe. -// -// It is implemented by Pipe and, through embedding, all other types. -type Sizer interface { - // PipeSize returns the pipe capacity in bytes. - PipeSize() int64 - - // SetPipeSize sets the new pipe capacity in bytes. - // - // The new size is returned (which may be capped). - SetPipeSize(int64) (int64, error) -} - // Pipe is an encapsulation of a platform-independent pipe. // It manages a buffered byte queue shared between a reader/writer // pair. @@ -399,15 +386,15 @@ func (p *Pipe) queued() int64 { return p.size } -// PipeSize implements PipeSizer.PipeSize. -func (p *Pipe) PipeSize() int64 { +// FifoSize implements fs.FifoSizer.FifoSize. +func (p *Pipe) FifoSize(context.Context, *fs.File) (int64, error) { p.mu.Lock() defer p.mu.Unlock() - return p.max + return p.max, nil } -// SetPipeSize implements PipeSize.SetPipeSize. -func (p *Pipe) SetPipeSize(size int64) (int64, error) { +// SetFifoSize implements fs.FifoSizer.SetFifoSize. +func (p *Pipe) SetFifoSize(size int64) (int64, error) { if size < 0 { return 0, syserror.EINVAL } diff --git a/pkg/sentry/kernel/pipe/reader_writer.go b/pkg/sentry/kernel/pipe/reader_writer.go index fedfcd921..f69dbf27b 100644 --- a/pkg/sentry/kernel/pipe/reader_writer.go +++ b/pkg/sentry/kernel/pipe/reader_writer.go @@ -77,7 +77,7 @@ func (rw *ReaderWriter) Readiness(mask waiter.EventMask) waiter.EventMask { } // Ioctl implements fs.FileOperations.Ioctl. -func (rw *ReaderWriter) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (rw *ReaderWriter) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // Switch on ioctl request. switch int(args[1].Int()) { case linux.FIONREAD: diff --git a/pkg/sentry/socket/epsocket/epsocket.go b/pkg/sentry/socket/epsocket/epsocket.go index 8e65e1b3f..2a38e370a 100644 --- a/pkg/sentry/socket/epsocket/epsocket.go +++ b/pkg/sentry/socket/epsocket/epsocket.go @@ -1993,7 +1993,7 @@ func (s *SocketOperations) SendMsg(t *kernel.Task, src usermem.IOSequence, to [] } // Ioctl implements fs.FileOperations.Ioctl. -func (s *SocketOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (s *SocketOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // SIOCGSTAMP is implemented by epsocket rather than all commonEndpoint // sockets. // TODO(b/78348848): Add a commonEndpoint method to support SIOCGSTAMP. diff --git a/pkg/sentry/socket/hostinet/socket_unsafe.go b/pkg/sentry/socket/hostinet/socket_unsafe.go index 7bd3a70c4..6c69ba9c7 100644 --- a/pkg/sentry/socket/hostinet/socket_unsafe.go +++ b/pkg/sentry/socket/hostinet/socket_unsafe.go @@ -20,6 +20,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/context" + "gvisor.dev/gvisor/pkg/sentry/fs" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/usermem" "gvisor.dev/gvisor/pkg/syserr" @@ -52,7 +53,7 @@ func writev(fd int, srcs []syscall.Iovec) (uint64, error) { } // Ioctl implements fs.FileOperations.Ioctl. -func (s *socketOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (s *socketOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { switch cmd := uintptr(args[1].Int()); cmd { case syscall.TIOCINQ, syscall.TIOCOUTQ: var val int32 diff --git a/pkg/sentry/socket/netlink/socket.go b/pkg/sentry/socket/netlink/socket.go index 87c0c77bc..ecc1e2d53 100644 --- a/pkg/sentry/socket/netlink/socket.go +++ b/pkg/sentry/socket/netlink/socket.go @@ -173,7 +173,7 @@ func (s *Socket) EventUnregister(e *waiter.Entry) { } // Ioctl implements fs.FileOperations.Ioctl. -func (s *Socket) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (*Socket) Ioctl(context.Context, *fs.File, usermem.IO, arch.SyscallArguments) (uintptr, error) { // TODO(b/68878065): no ioctls supported. return 0, syserror.ENOTTY } diff --git a/pkg/sentry/socket/rpcinet/socket.go b/pkg/sentry/socket/rpcinet/socket.go index c76b48ead..cc7b964ea 100644 --- a/pkg/sentry/socket/rpcinet/socket.go +++ b/pkg/sentry/socket/rpcinet/socket.go @@ -564,7 +564,7 @@ func ifconfIoctlFromStack(ctx context.Context, io usermem.IO, ifc *linux.IFConf) } // Ioctl implements fs.FileOperations.Ioctl. -func (s *socketOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (s *socketOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { t := ctx.(*kernel.Task) cmd := uint32(args[1].Int()) diff --git a/pkg/sentry/socket/unix/unix.go b/pkg/sentry/socket/unix/unix.go index 5fc43db8c..6190de0c5 100644 --- a/pkg/sentry/socket/unix/unix.go +++ b/pkg/sentry/socket/unix/unix.go @@ -152,7 +152,7 @@ func (s *SocketOperations) GetSockName(t *kernel.Task) (interface{}, uint32, *sy } // Ioctl implements fs.FileOperations.Ioctl. -func (s *SocketOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { +func (s *SocketOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { return epsocket.Ioctl(ctx, s.ep, io, args) } diff --git a/pkg/sentry/syscalls/linux/sys_file.go b/pkg/sentry/syscalls/linux/sys_file.go index 04962726a..3ef7441c2 100644 --- a/pkg/sentry/syscalls/linux/sys_file.go +++ b/pkg/sentry/syscalls/linux/sys_file.go @@ -27,7 +27,6 @@ import ( "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/fasync" "gvisor.dev/gvisor/pkg/sentry/kernel/kdefs" - "gvisor.dev/gvisor/pkg/sentry/kernel/pipe" ktime "gvisor.dev/gvisor/pkg/sentry/kernel/time" "gvisor.dev/gvisor/pkg/sentry/limits" "gvisor.dev/gvisor/pkg/sentry/usermem" @@ -627,7 +626,7 @@ func Ioctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall return 0, nil, err default: - ret, err := file.FileOperations.Ioctl(t, t.MemoryManager(), args) + ret, err := file.FileOperations.Ioctl(t, file, t.MemoryManager(), args) if err != nil { return 0, nil, err } @@ -1000,17 +999,18 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall err := tmpfs.AddSeals(file.Dirent.Inode, args[2].Uint()) return 0, nil, err case linux.F_GETPIPE_SZ: - sz, ok := file.FileOperations.(pipe.Sizer) + sz, ok := file.FileOperations.(fs.FifoSizer) if !ok { return 0, nil, syserror.EINVAL } - return uintptr(sz.PipeSize()), nil, nil + size, err := sz.FifoSize(t, file) + return uintptr(size), nil, err case linux.F_SETPIPE_SZ: - sz, ok := file.FileOperations.(pipe.Sizer) + sz, ok := file.FileOperations.(fs.FifoSizer) if !ok { return 0, nil, syserror.EINVAL } - n, err := sz.SetPipeSize(int64(args[2].Int())) + n, err := sz.SetFifoSize(int64(args[2].Int())) return uintptr(n), nil, err default: // Everything else is not yet supported. diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index b06e46c03..b5f89033b 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -255,7 +255,7 @@ syscall_test(test = "//test/syscalls/linux:pause_test") syscall_test( size = "large", - add_overlay = False, # TODO(gvisor.dev/issue/318): enable when fixed. + add_overlay = True, shard_count = 5, test = "//test/syscalls/linux:pipe_test", ) diff --git a/test/syscalls/linux/pipe.cc b/test/syscalls/linux/pipe.cc index 1f4422f4e..65afb90f3 100644 --- a/test/syscalls/linux/pipe.cc +++ b/test/syscalls/linux/pipe.cc @@ -339,11 +339,13 @@ TEST_P(PipeTest, BlockPartialWriteClosed) { SKIP_IF(!CreateBlocking()); ScopedThread t([this]() { - std::vector buf(2 * Size()); + const int pipe_size = Size(); + std::vector buf(2 * pipe_size); + // Write more than fits in the buffer. Blocks then returns partial write // when the other end is closed. The next call returns EPIPE. ASSERT_THAT(write(wfd_.get(), buf.data(), buf.size()), - SyscallSucceedsWithValue(Size())); + SyscallSucceedsWithValue(pipe_size)); EXPECT_THAT(write(wfd_.get(), buf.data(), buf.size()), SyscallFailsWithErrno(EPIPE)); });