From d94cb4a5563d03e38f0b0d5e62ee1744c091cfbb Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 28 Oct 2026 17:12:47 -0400 Subject: [PATCH] Preserve the full counter when converting an eventfd to a host fd A guest can write more than 32 bits to an eventfd before a device needs the underlying Linux descriptor. eventfd2 takes an unsigned int, so passing the accumulated counter as its initial value lost the high bits. Seed the low bits at creation, write any remaining amount while the descriptor is still private, and clear the guest counter only after the transfer succeeds. --- pkg/sentry/fsimpl/eventfd/BUILD | 2 + pkg/sentry/fsimpl/eventfd/eventfd.go | 36 ++++++++----- pkg/sentry/fsimpl/eventfd/eventfd_test.go | 62 +++++++++++++++++++++++ 3 files changed, 86 insertions(+), 14 deletions(-) diff --git a/pkg/sentry/fsimpl/eventfd/BUILD b/pkg/sentry/fsimpl/eventfd/BUILD index db86873e0d3..f68ca99307e 100644 --- a/pkg/sentry/fsimpl/eventfd/BUILD +++ b/pkg/sentry/fsimpl/eventfd/BUILD @@ -34,9 +34,11 @@ go_test( embed = [":eventfd"], deps = [ "//pkg/abi/linux", + "//pkg/hostarch", "//pkg/sentry/contexttest", "//pkg/sentry/vfs", "//pkg/usermem", "//pkg/waiter", + "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/sentry/fsimpl/eventfd/eventfd.go b/pkg/sentry/fsimpl/eventfd/eventfd.go index 45bfca18e75..462c5c4fe74 100644 --- a/pkg/sentry/fsimpl/eventfd/eventfd.go +++ b/pkg/sentry/fsimpl/eventfd/eventfd.go @@ -60,6 +60,7 @@ type EventFileDescription struct { semMode bool // hostfd is the backing host eventfd, or -1 if none is installed. + // A reference to vfsfd keeps an installed fd open after mu is released. // // +checklocks:mu hostfd int @@ -134,19 +135,28 @@ func (efd *EventFileDescription) HostFD() (int, error) { flags |= linux.EFD_SEMAPHORE } - fd, _, errno := unix.Syscall(unix.SYS_EVENTFD2, uintptr(efd.val), uintptr(flags), 0) - if errno != 0 { - return -1, errno + // eventfd2 accepts a 32-bit initial value, but writes can raise the + // counter above that before a device asks for a host fd. + initVal := uint32(efd.val) + fd, err := unix.Eventfd(uint(initVal), flags) + if err != nil { + return -1, err } - - if err := fdnotifier.AddFD(int32(fd), &efd.queue); err != nil { - if closeErr := unix.Close(int(fd)); closeErr != nil { + if rest := efd.val - uint64(initVal); rest != 0 { + err = hostWrite(fd, rest) + } + if err == nil { + err = fdnotifier.AddFD(int32(fd), &efd.queue) + } + if err != nil { + if closeErr := unix.Close(fd); closeErr != nil { log.Warningf("close(%d) eventfd failed: %v", fd, closeErr) } return -1, err } - efd.hostfd = int(fd) + efd.val = 0 + efd.hostfd = fd efd.sentryOwnedHostfd = true return efd.hostfd, nil } @@ -247,11 +257,10 @@ func (efd *EventFileDescription) read(ctx context.Context, dst usermem.IOSequenc return err } -// +checklocks:efd.mu -func (efd *EventFileDescription) hostWriteLocked(val uint64) error { +func hostWrite(fd int, val uint64) error { var buf [8]byte hostarch.ByteOrder.PutUint64(buf[:], val) - _, err := unix.Write(efd.hostfd, buf[:]) + _, err := unix.Write(fd, buf[:]) if err == unix.EWOULDBLOCK { return linuxerr.ErrWouldBlock } @@ -280,10 +289,9 @@ func (efd *EventFileDescription) Signal(val uint64) error { } efd.mu.Lock() - - if efd.hostfd >= 0 { - defer efd.mu.Unlock() - return efd.hostWriteLocked(val) + if fd := efd.hostfd; fd >= 0 { + efd.mu.Unlock() + return hostWrite(fd, val) } // We only allow writes that won't cause the value to go over the max diff --git a/pkg/sentry/fsimpl/eventfd/eventfd_test.go b/pkg/sentry/fsimpl/eventfd/eventfd_test.go index 41b4cd7f6e1..d8e92856efe 100644 --- a/pkg/sentry/fsimpl/eventfd/eventfd_test.go +++ b/pkg/sentry/fsimpl/eventfd/eventfd_test.go @@ -15,9 +15,12 @@ package eventfd import ( + "math" "testing" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/sentry/contexttest" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/usermem" @@ -97,3 +100,62 @@ func TestEventFDStat(t *testing.T) { t.Errorf("eventfd size should be 0") } } + +func TestHostFDPreservesCounter(t *testing.T) { + for _, tc := range []struct { + name string + count uint64 + }{ + {"max_uint32", math.MaxUint32}, + {"above_uint32", 1<<32 + 1}, + {"max_eventfd", math.MaxUint64 - 1}, + } { + t.Run(tc.name+"/counter", func(t *testing.T) { testHostFDCounter(t, tc.count, false) }) + t.Run(tc.name+"/semaphore", func(t *testing.T) { testHostFDCounter(t, tc.count, true) }) + } +} + +func testHostFDCounter(t *testing.T, count uint64, semaphore bool) { + ctx := contexttest.Context(t) + var vfsObj vfs.VirtualFilesystem + if err := vfsObj.Init(ctx); err != nil { + t.Fatal(err) + } + defer vfsObj.Release(ctx) + file, err := New(ctx, &vfsObj, 1, semaphore, linux.O_RDWR) + if err != nil { + t.Fatal(err) + } + defer file.DecRef(ctx) + var buf [8]byte + hostarch.ByteOrder.PutUint64(buf[:], count-1) + if _, err := file.Write(ctx, usermem.BytesIOSequence(buf[:]), vfs.WriteOptions{}); err != nil { + t.Fatal(err) + } + fd, err := file.Impl().(*EventFileDescription).HostFD() + if err != nil { + t.Fatal(err) + } + if _, err := unix.Read(fd, buf[:]); err != nil { + t.Fatalf("read host eventfd: %v", err) + } + want := count + if semaphore { + want = 1 + } + if got := hostarch.ByteOrder.Uint64(buf[:]); got != want { + t.Fatalf("read host eventfd: got %d, want %d", got, want) + } + if semaphore { + // A single read consumes one. Fill the remaining space + // to verify the exact count without draining it. + hostarch.ByteOrder.PutUint64(buf[:], math.MaxUint64-count) + if _, err := unix.Write(fd, buf[:]); err != nil { + t.Fatalf("fill host semaphore: %v", err) + } + hostarch.ByteOrder.PutUint64(buf[:], 1) + if _, err := unix.Write(fd, buf[:]); err != unix.EAGAIN { + t.Fatalf("write to full host semaphore: got %v, want EAGAIN", err) + } + } +}