Skip to content

Commit 704543b

Browse files
nixprimegvisor-bot
authored andcommitted
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 google#9736 PiperOrigin-RevId: 584091971
1 parent 88d35cd commit 704543b

File tree

3 files changed

+21
-1
lines changed

3 files changed

+21
-1
lines changed

g3doc/user_guide/compatibility.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ Most common utilities work. Note that:
6767
| gcore | Working. |
6868
| gdb | Working. |
6969
| gosu | Working. |
70-
| grep | Working (unless stdin is a pipe and stdout is /dev/null). |
70+
| grep | Working. |
7171
| ifconfig | Works partially, like ip. Full support [in progress](https://gvisor.dev/issue/578). |
7272
| ip | Some subcommands work (e.g. addr, route). Full support [in progress](https://gvisor.dev/issue/578). |
7373
| less | Working. |

pkg/sentry/kernel/pipe/vfs.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,13 @@ func (fd *VFSPipeFD) SpliceToNonPipe(ctx context.Context, out *vfs.FileDescripti
277277
fd.pipe.consumeLocked(n)
278278
}
279279

280+
// Implementations of out.[P]Write() that ignore written data (e.g.
281+
// /dev/null) may skip calling src.CopyIn[To]() and therefore miss getting
282+
// ErrWouldBlock from Pipe.peekLocked().
283+
if n == 0 && err == nil && fd.pipe.size == 0 && fd.pipe.HasWriters() {
284+
err = linuxerr.ErrWouldBlock
285+
}
286+
280287
fd.pipe.mu.Unlock()
281288

282289
if n > 0 {

test/syscalls/linux/splice.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,19 @@ TEST(SpliceTest, FromPipeWithConcurrentIo) {
921921
}
922922
}
923923

924+
// Regression test for #9736.
925+
TEST(SpliceTest, FromEmptyPipeWithWriterToDevNull) {
926+
int fds[2];
927+
ASSERT_THAT(pipe(fds), SyscallSucceeds());
928+
const FileDescriptor rfd(fds[0]);
929+
const FileDescriptor wfd(fds[1]);
930+
const FileDescriptor out_fd =
931+
ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/null", O_WRONLY));
932+
ASSERT_THAT(
933+
splice(rfd.get(), nullptr, out_fd.get(), nullptr, 1, SPLICE_F_NONBLOCK),
934+
SyscallFailsWithErrno(EAGAIN));
935+
}
936+
924937
} // namespace
925938

926939
} // namespace testing

0 commit comments

Comments
 (0)