Fix #10046

See updated comment in sentry/kernel/pipe/vfs.go.

PiperOrigin-RevId: 610516821

pkg/sentry/kernel/pipe/pipe.go

@@ -179,9 +179,9 @@ func initPipe(pipe *Pipe, isNamed bool, sizeBytes int64) {
 	pipe.max = sizeBytes
 }
 
-// peekLocked passes the first count bytes in the pipe to f and returns its
-// result. If fewer than count bytes are available, the safemem.BlockSeq passed
-// to f will be less than count bytes in length.
+// peekLocked passes the first count bytes in the pipe, starting at offset off,
+// to f and returns its result. If fewer than count bytes are available, the
+// safemem.BlockSeq passed to f will be less than count bytes in length.
 //
 // peekLocked does not mutate the pipe; if the read consumes bytes from the
 // pipe, then the caller is responsible for calling p.consumeLocked() and
@@ -191,25 +191,30 @@ func initPipe(pipe *Pipe, isNamed bool, sizeBytes int64) {
 // Preconditions:
 //   - p.mu must be locked.
 //   - This pipe must have readers.
-func (p *Pipe) peekLocked(count int64, f func(safemem.BlockSeq) (uint64, error)) (int64, error) {
+//   - off <= p.size.
+func (p *Pipe) peekLocked(off, count int64, f func(safemem.BlockSeq) (uint64, error)) (int64, error) {
 	// Don't block for a zero-length read even if the pipe is empty.
 	if count == 0 {
 		return 0, nil
 	}
 
 	// Limit the amount of data read to the amount of data in the pipe.
-	if count > p.size {
-		if p.size == 0 {
+	if rem := p.size - off; count > rem {
+		if rem == 0 {
 			if !p.HasWriters() {
 				return 0, io.EOF
 			}
 			return 0, linuxerr.ErrWouldBlock
 		}
-		count = p.size
+		count = rem
 	}
 
 	// Prepare the view of the data to be read.
-	bs := p.bufBlockSeq.DropFirst64(uint64(p.off)).TakeFirst64(uint64(count))
+	pipeOff := p.off + off
+	if max := int64(len(p.buf)); pipeOff >= max {
+		pipeOff -= max
+	}
+	bs := p.bufBlockSeq.DropFirst64(uint64(pipeOff)).TakeFirst64(uint64(count))
 
 	// Perform the read.
 	done, err := f(bs)

pkg/sentry/kernel/pipe/pipe_util.go

@@ -66,7 +66,7 @@ func (p *Pipe) Read(ctx context.Context, dst usermem.IOSequence) (int64, error)
 func (p *Pipe) read(count int64, f func(srcs safemem.BlockSeq) (uint64, error), removeFromSrc bool) (int64, error) {
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	n, err := p.peekLocked(count, f)
+	n, err := p.peekLocked(0, count, f)
 	if n > 0 && removeFromSrc {
 		p.consumeLocked(n)
 	}

pkg/sentry/kernel/pipe/vfs.go

@@ -279,10 +279,13 @@ func (fd *VFSPipeFD) SpliceToNonPipe(ctx context.Context, out *vfs.FileDescripti
 	} else {
 		n, err = out.PWrite(ctx, src, off, vfs.WriteOptions{})
 	}
-
 	// Implementations of out.[P]Write() that ignore written data (e.g.
-	// /dev/null) may skip calling src.CopyIn[To]() and therefore miss getting
-	// ErrWouldBlock from Pipe.peekLocked().
+	// /dev/null) may skip calling src.CopyIn[To](), so:
+	//
+	// - We must call Pipe.consumeLocked() here rather than in fd.CopyIn[To]().
+	//
+	// - We must check if Pipe.peekLocked() would have returned ErrWouldBlock.
+	fd.pipe.consumeLocked(n)
 	if n == 0 && err == nil && fd.pipe.size == 0 && fd.pipe.HasWriters() {
 		err = linuxerr.ErrWouldBlock
 	}
@@ -331,10 +334,9 @@ func (fd *VFSPipeFD) CopyIn(ctx context.Context, addr hostarch.Addr, dst []byte,
 		log.Traceback("Non-sequential VFSPipeFD.CopyIn: lastAddr=%#x addr=%#x", fd.lastAddr, addr)
 		return 0, linuxerr.EINVAL
 	}
-	n, err := fd.pipe.peekLocked(int64(len(dst)), func(srcs safemem.BlockSeq) (uint64, error) {
+	n, err := fd.pipe.peekLocked(int64(addr), int64(len(dst)), func(srcs safemem.BlockSeq) (uint64, error) {
 		return safemem.CopySeq(safemem.BlockSeqOf(safemem.BlockFromSafeSlice(dst)), srcs)
 	})
-	fd.pipe.consumeLocked(n)
 	fd.lastAddr = addr + hostarch.Addr(n)
 	return int(n), err
 }
@@ -384,10 +386,9 @@ func (fd *VFSPipeFD) CopyInTo(ctx context.Context, ars hostarch.AddrRangeSeq, ds
 			log.Traceback("Non-sequential VFSPipeFD.CopyInTo: lastAddr=%#x addr=%#x", fd.lastAddr, ar.Start)
 			return total, linuxerr.EINVAL
 		}
-		n, err := fd.pipe.peekLocked(int64(ar.Length()), func(srcs safemem.BlockSeq) (uint64, error) {
+		n, err := fd.pipe.peekLocked(int64(ar.Start), int64(ar.Length()), func(srcs safemem.BlockSeq) (uint64, error) {
 			return dst.WriteFromBlocks(srcs)
 		})
-		fd.pipe.consumeLocked(n)
 		fd.lastAddr = ar.Start + hostarch.Addr(n)
 		total += n
 		if err != nil {
@@ -464,7 +465,7 @@ func spliceOrTee(ctx context.Context, dst, src *VFSPipeFD, count int64, removeFr
 
 	firstLocked, secondLocked := lockTwoPipes(dst.pipe, src.pipe)
 	n, err := dst.pipe.writeLocked(count, func(dsts safemem.BlockSeq) (uint64, error) {
-		n, err := src.pipe.peekLocked(int64(dsts.NumBytes()), func(srcs safemem.BlockSeq) (uint64, error) {
+		n, err := src.pipe.peekLocked(0, int64(dsts.NumBytes()), func(srcs safemem.BlockSeq) (uint64, error) {
 			return safemem.CopySeq(dsts, srcs)
 		})
 		if n > 0 && removeFromSrc {

test/syscalls/linux/splice.cc

@@ -921,14 +921,26 @@ TEST(SpliceTest, FromPipeWithConcurrentIo) {
   }
 }
 
-// Regression test for #9736.
-TEST(SpliceTest, FromEmptyPipeWithWriterToDevNull) {
+// Regression test for #9736 and #10046.
+TEST(SpliceTest, FromPipeWithWriterToDevNull) {
   int fds[2];
   ASSERT_THAT(pipe(fds), SyscallSucceeds());
   const FileDescriptor rfd(fds[0]);
   const FileDescriptor wfd(fds[1]);
   const FileDescriptor out_fd =
       ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/null", O_WRONLY));
+
+  // Write one byte to the pipe and expect it to be successfully spliced to
+  // /dev/null.
+  constexpr char kSplicedByte = '.';
+  ASSERT_THAT(WriteFd(wfd.get(), &kSplicedByte, 1),
+              SyscallSucceedsWithValue(1));
+  ASSERT_THAT(
+      splice(rfd.get(), nullptr, out_fd.get(), nullptr, 1, SPLICE_F_NONBLOCK),
+      SyscallSucceedsWithValue(1));
+
+  // Expect that splicing from the pipe again fails with EAGAIN since the pipe
+  // is now empty.
   ASSERT_THAT(
       splice(rfd.get(), nullptr, out_fd.get(), nullptr, 1, SPLICE_F_NONBLOCK),
       SyscallFailsWithErrno(EAGAIN));