Complete pipe support on overlayfs

Get/Set pipe size and ioctl support were missing from
overlayfs. It required moving the pipe.Sizer interface
to fs so that overlay could get access.

Fixes #318

PiperOrigin-RevId: 255511125

19 files changed, 85 insertions, 44 deletions

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<char> buf(2 * Size());
+    const int pipe_size = Size();
+    std::vector<char> 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));
   });