Ensure empty VFSPipeFD.SpliceToNonPipe(/dev/null) returns ErrWouldBlock.

When grep's stdout is /dev/null (so printed matches are discarded), its outcome
is only observable in its exit code, which is binary (0 for matches, 1 for no
matches). When grep's stdin is additionally a pipe, GNU grep optimizes for this
specific case by switching from reading input to splicing it directly to stdout
after the first match:

```
  if (exit_on_match | dev_null_output)
    list_files = LISTFILES_NONE;
...
  if (list_files == LISTFILES_NONE)
    finalize_input (desc, &st, ineof);
...
static bool
drain_input (int fd, struct stat const *st)
{
  ssize_t nbytes;
  if (S_ISFIFO (st->st_mode) && dev_null_output)
    {
#ifdef SPLICE_F_MOVE
      /* Should be faster, since it need not copy data to user space.  */
      nbytes = splice (fd, NULL, STDOUT_FILENO, NULL,
                       INITIAL_BUFSIZE, SPLICE_F_MOVE);
```

This triggers a bug in our splice implementation: since memdev.nullFD.Write()
never calls back into pipe.Pipe.peekLocked() to get ErrWouldBlock, this is
never propagated up to syscalls/linux.Splice(). Consequently, splice() returns
0 instead of blocking; grep interprets this as EOF from the pipe and exits.

We can't fix this by calling src.CopyInTo() in memdev.nullFD.Write() because
this would have the wrong behavior for `write(/dev/null, unmapped addr)`, which
should succeed because `drivers/char/mem.c:null_write()` also ignores the
application-provided pointer. Instead, handle this in
VFSPipeFD.SpliceToNonPipe(). (Linux instead avoids this problem by
distinguishing file_operations::write and file_operations::splice_write, which
we would prefer to avoid if possible.)

Fixes #9736

PiperOrigin-RevId: 584091971
This commit is contained in:
Jamie Liu
2023-11-20 12:04:52 -08:00
committed by gVisor bot
parent 88d35cd8f1
commit 704543b8f3
3 changed files with 21 additions and 1 deletions
+1 -1
View File
@@ -67,7 +67,7 @@ Most common utilities work. Note that:
| gcore | Working. |
| gdb | Working. |
| gosu | Working. |
| grep | Working (unless stdin is a pipe and stdout is /dev/null). |
| grep | Working. |
| ifconfig | Works partially, like ip. Full support [in progress](https://gvisor.dev/issue/578). |
| ip | Some subcommands work (e.g. addr, route). Full support [in progress](https://gvisor.dev/issue/578). |
| less | Working. |
+7
View File
@@ -277,6 +277,13 @@ func (fd *VFSPipeFD) SpliceToNonPipe(ctx context.Context, out *vfs.FileDescripti
fd.pipe.consumeLocked(n)
}
// 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().
if n == 0 && err == nil && fd.pipe.size == 0 && fd.pipe.HasWriters() {
err = linuxerr.ErrWouldBlock
}
fd.pipe.mu.Unlock()
if n > 0 {
+13
View File
@@ -921,6 +921,19 @@ TEST(SpliceTest, FromPipeWithConcurrentIo) {
}
}
// Regression test for #9736.
TEST(SpliceTest, FromEmptyPipeWithWriterToDevNull) {
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));
ASSERT_THAT(
splice(rfd.get(), nullptr, out_fd.get(), nullptr, 1, SPLICE_F_NONBLOCK),
SyscallFailsWithErrno(EAGAIN));
}
} // namespace
} // namespace testing