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) + } + } +}