poll: Write back only revents to userspace, matching Linux behavior

doPoll() was modifying pfd.Events (adding POLLHUP|POLLERR) and writing
back the entire pollfd struct. Linux only writes back revents via
unsafe_put_user() in do_sys_poll(). The polluted events field broke
libevent's poll_del(), causing a busy-loop in tmux (CPU 96.6%).

Fix: use an internal mask for POLLHUP|POLLERR in initReadiness() and
pollBlock(); write back only the 2-byte revents field per fd in doPoll().

Test: PollTest.EventsFieldNotModified, PollTest.EventsFieldNotModifiedOnError
Signed-off-by: Tan Yifeng <yiftan@tencent.com>

Author: tanyifeng

pkg/sentry/syscalls/linux/sys_poll.go

@@ -21,6 +21,7 @@ import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/hostarch"
+	"gvisor.dev/gvisor/pkg/marshal/primitive"
 	"gvisor.dev/gvisor/pkg/sentry/arch"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/ktime"
@@ -34,6 +35,16 @@ import (
 // unrecoverable.
 const fileCap = 1024 * 1024
 
+var (
+	// sizeofPollFD is the size of linux.PollFD struct in bytes.
+	sizeofPollFD = (*linux.PollFD)(nil).SizeBytes()
+
+	// reventsOffsetInFD is the byte offset of the REvents field within
+	// linux.PollFD. Since REvents is the last field (int16), its offset
+	// equals the struct size minus the size of int16.
+	reventsOffsetInFD = sizeofPollFD - (*primitive.Int16)(nil).SizeBytes()
+)
+
 // Masks for "readable", "writable", and "exceptional" events as defined by
 // select(2).
 const (
@@ -72,18 +83,24 @@ func initReadiness(t *kernel.Task, pfd *linux.PollFD, state *pollState, ch chan
 		return nil
 	}
 
+	// Like Linux's fs/select.c:do_pollfd(), always check for POLLHUP and
+	// POLLERR in addition to the caller-requested events. We add these
+	// to a local mask rather than modifying pfd.Events, because Linux
+	// never writes back the events field to userspace (only revents).
+	mask := pfd.Events | linux.POLLHUP | linux.POLLERR
+
 	if ch == nil {
 		defer file.DecRef(t)
 	} else {
 		state.file = file
-		state.waiter.Init(waiter.ChannelNotifier(ch), waiter.EventMaskFromLinux(uint32(pfd.Events)))
+		state.waiter.Init(waiter.ChannelNotifier(ch), waiter.EventMaskFromLinux(uint32(mask)))
 		if err := file.EventRegister(&state.waiter); err != nil {
 			return err
 		}
 	}
 
-	r := file.Readiness(waiter.EventMaskFromLinux(uint32(pfd.Events)))
-	pfd.REvents = int16(r.ToLinux()) & pfd.Events
+	r := file.Readiness(waiter.EventMaskFromLinux(uint32(mask)))
+	pfd.REvents = int16(r.ToLinux()) & mask
 	return nil
 }
 
@@ -150,8 +167,9 @@ func pollBlock(t *kernel.Task, pfd []linux.PollFD, timeout time.Duration) (time.
 				continue
 			}
 
-			r := state[i].file.Readiness(waiter.EventMaskFromLinux(uint32(pfd[i].Events)))
-			rl := int16(r.ToLinux()) & pfd[i].Events
+			mask := pfd[i].Events | linux.POLLHUP | linux.POLLERR
+			r := state[i].file.Readiness(waiter.EventMaskFromLinux(uint32(mask)))
+			rl := int16(r.ToLinux()) & mask
 			if rl != 0 {
 				pfd[i].REvents = rl
 				n++
@@ -184,21 +202,23 @@ func doPoll(t *kernel.Task, addr hostarch.Addr, nfds uint, timeout time.Duration
 		return timeout, 0, err
 	}
 
-	// Compatibility warning: Linux adds POLLHUP and POLLERR just before
-	// polling, in fs/select.c:do_pollfd(). Since pfd is copied out after
-	// polling, changing event masks here is an application-visible difference.
-	// (Linux also doesn't copy out event masks at all, only revents.)
-	for i := range pfd {
-		pfd[i].Events |= linux.POLLHUP | linux.POLLERR
-	}
 	remainingTimeout, n, err := pollBlock(t, pfd, timeout)
 	err = linuxerr.ConvertIntr(err, linuxerr.EINTR)
 
-	// The poll entries are copied out regardless of whether
-	// any are set or not. This aligns with the Linux behavior.
+	// Copy out only the revents field, matching Linux behavior.
+	// Linux's do_sys_poll() only writes back revents via
+	// unsafe_put_user(fds->revents, &ufds->revents), never the
+	// events field. Writing back the full struct would corrupt
+	// the caller's events mask (e.g. libevent's poll backend),
+	// causing busy-loops when event_del() fails to fully remove
+	// an fd from the pollfd array due to stale POLLHUP/POLLERR bits.
+	//
 	if nfds > 0 && err == nil {
-		if _, err := linux.CopyPollFDSliceOut(t, addr, pfd); err != nil {
-			return remainingTimeout, 0, err
+		for i := range pfd {
+			off := hostarch.Addr(i*sizeofPollFD + reventsOffsetInFD)
+			if _, copyErr := primitive.CopyInt16Out(t, addr+off, pfd[i].REvents); copyErr != nil {
+				return remainingTimeout, 0, copyErr
+			}
 		}
 	}
 

test/syscalls/linux/poll.cc

@@ -364,6 +364,55 @@ TEST_F(PollTest, UnpollableFile) {
   EXPECT_EQ(poll_fd.revents, POLLIN | POLLOUT);
 }
 
+// Test that poll(2) does not write back the events field to userspace.
+// Linux's do_sys_poll() only writes back revents via unsafe_put_user(),
+// never the events field. This verifies that POLLHUP/POLLERR are not
+// injected into the events field.
+TEST_F(PollTest, EventsFieldNotModified) {
+  // Create a pipe.
+  int fds[2];
+  ASSERT_THAT(pipe(fds), SyscallSucceeds());
+
+  FileDescriptor fd0(fds[0]);
+  FileDescriptor fd1(fds[1]);
+
+  // Close the writer fd so the reader gets POLLHUP.
+  fd1.reset();
+
+  // Poll with only POLLIN in events.
+  struct pollfd poll_fd = {fd0.get(), POLLIN, 0};
+  EXPECT_THAT(RetryEINTR(poll)(&poll_fd, 1, 0), SyscallSucceedsWithValue(1));
+
+  // revents should contain POLLHUP (since writer is closed).
+  EXPECT_NE(poll_fd.revents & POLLHUP, 0);
+
+  // The events field must remain unchanged (only POLLIN, no POLLHUP/POLLERR).
+  EXPECT_EQ(poll_fd.events, POLLIN);
+}
+
+// Test that poll(2) does not write back the events field when POLLERR occurs.
+TEST_F(PollTest, EventsFieldNotModifiedOnError) {
+  // Create a pipe.
+  int fds[2];
+  ASSERT_THAT(pipe(fds), SyscallSucceeds());
+
+  FileDescriptor fd0(fds[0]);
+  FileDescriptor fd1(fds[1]);
+
+  // Close the reader fd so the writer gets POLLERR.
+  fd0.reset();
+
+  // Poll with only POLLOUT in events.
+  struct pollfd poll_fd = {fd1.get(), POLLOUT, 0};
+  EXPECT_THAT(RetryEINTR(poll)(&poll_fd, 1, 0), SyscallSucceedsWithValue(1));
+
+  // revents should contain POLLERR (since reader is closed).
+  EXPECT_NE(poll_fd.revents & POLLERR, 0);
+
+  // The events field must remain unchanged (only POLLOUT, no POLLERR/POLLHUP).
+  EXPECT_EQ(poll_fd.events, POLLOUT);
+}
+
 }  // namespace
 }  // namespace testing
 }  // namespace gvisor