diff --git a/pkg/sentry/state/stateio/BUILD b/pkg/sentry/state/stateio/BUILD index f444827ffa..9747758492 100644 --- a/pkg/sentry/state/stateio/BUILD +++ b/pkg/sentry/state/stateio/BUILD @@ -25,6 +25,7 @@ go_library( deps = [ "//pkg/abi/linux", "//pkg/aio", + "//pkg/atomicbitops", "//pkg/hostarch", "//pkg/log", "//pkg/memutil", @@ -41,6 +42,7 @@ go_test( srcs = [ "bufreader_test.go", "bufwriter_test.go", + "fdwriter_test.go", "ioreader_test.go", "iowriter_test.go", ], diff --git a/pkg/sentry/state/stateio/fdwriter.go b/pkg/sentry/state/stateio/fdwriter.go index eec33510ac..c6fa0db055 100644 --- a/pkg/sentry/state/stateio/fdwriter.go +++ b/pkg/sentry/state/stateio/fdwriter.go @@ -20,6 +20,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/aio" + "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/hostfd" "gvisor.dev/gvisor/pkg/sentry/memmap" @@ -29,8 +30,8 @@ import ( type FDWriter struct { NoRegisterClientFD - // fd is the file descriptor. fd is immutable. - fd int32 + // fd is the file descriptor. + fd atomicbitops.Int32 // maxWriteBytes and maxRanges are AsyncWriter parameters. Both are // immutable. @@ -126,7 +127,7 @@ func NewFDWriter(fd int32, maxWriteBytes uint64, maxRanges, maxParallel int) *FD } return &FDWriter{ - fd: fd, + fd: atomicbitops.FromInt32(fd), maxWriteBytes: uint32(min(maxWriteBytes, uint64(linux.MAX_RW_COUNT))), maxRanges: uint32(min(maxRanges, hostfd.MaxReadWriteIov)), preextend: preextend, @@ -138,8 +139,12 @@ func NewFDWriter(fd int32, maxWriteBytes uint64, maxRanges, maxParallel int) *FD // Close implements AsyncWriter.Close. func (w *FDWriter) Close() error { + fd := w.fd.Swap(-1) + if fd == -1 { + return nil + } w.q.Destroy() - return unix.Close(int(w.fd)) + return unix.Close(int(fd)) } // MaxWriteBytes implements AsyncWriter.MaxWriteBytes. @@ -159,7 +164,7 @@ func (w *FDWriter) MaxParallel() int { // AddWrite implements AsyncWriter.AddWrite. func (w *FDWriter) AddWrite(id int, _ SourceFile, _ memmap.FileRange, srcMap []byte) { - aio.Write(w.q, uint64(id), w.fd, w.off, srcMap) + aio.Write(w.q, uint64(id), w.fd.Load(), w.off, srcMap) w.inflight[id] = fdWrite{ off: w.off, total: uint64(len(srcMap)), @@ -170,7 +175,7 @@ func (w *FDWriter) AddWrite(id int, _ SourceFile, _ memmap.FileRange, srcMap []b // AddWritev implements AsyncWriter.AddWritev. func (w *FDWriter) AddWritev(id int, total uint64, _ SourceFile, _ []memmap.FileRange, srcMaps []unix.Iovec) { - aio.Writev(w.q, uint64(id), w.fd, w.off, srcMaps) + aio.Writev(w.q, uint64(id), w.fd.Load(), w.off, srcMaps) w.inflight[id] = fdWrite{ off: w.off, total: total, @@ -186,10 +191,10 @@ func (w *FDWriter) Wait(cs []Completion, minCompletions int) ([]Completion, erro if w.preextend && w.fileSize < w.off { newSize := max(w.off, w.reserved) w.reserved = 0 - if err := unix.Ftruncate(int(w.fd), newSize); err != nil { + if err := unix.Ftruncate(int(w.fd.Load()), newSize); err != nil { // This can occur if e.g. the file is FUSE-backed, and the FUSE // server doesn't support file extension. - log.Infof("stateio.FDWriter: ftruncate(%d, %d) failed: %v", w.fd, newSize, err) + log.Infof("stateio.FDWriter: ftruncate(%d, %d) failed: %v", w.fd.Load(), newSize, err) w.preextend = false // Update w.fileSize assuming that all writes complete // successfully, as below. @@ -240,9 +245,9 @@ retry: inflight.done = done inflight.src = inflight.src.DropFirst(n) if inflight.src.Mapping != nil { - aio.Write(w.q, aioC.ID, w.fd, inflight.off, inflight.src.Mapping) + aio.Write(w.q, aioC.ID, w.fd.Load(), inflight.off, inflight.src.Mapping) } else { - aio.Writev(w.q, aioC.ID, w.fd, inflight.off, inflight.src.Iovecs) + aio.Writev(w.q, aioC.ID, w.fd.Load(), inflight.off, inflight.src.Iovecs) } // w.q may be an aio.LinuxQueue, in which case we need to call // w.q.Wait() again to submit this write. diff --git a/pkg/sentry/state/stateio/fdwriter_test.go b/pkg/sentry/state/stateio/fdwriter_test.go new file mode 100644 index 0000000000..c65aef7edd --- /dev/null +++ b/pkg/sentry/state/stateio/fdwriter_test.go @@ -0,0 +1,45 @@ +// Copyright 2026 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package stateio + +import ( + "testing" + + "golang.org/x/sys/unix" +) + +func TestFDWriterDoubleClose(t *testing.T) { + var fds [2]int + if err := unix.Pipe(fds[:]); err != nil { + t.Fatalf("Pipe failed: %v", err) + } + readFD := fds[0] + writeFD := fds[1] + + defer unix.Close(readFD) + + // NewFDWriter takes ownership of writeFD. + w := NewFDWriter(int32(writeFD), 4096, 1, 1) + + // First close should succeed. + if err := w.Close(); err != nil { + t.Fatalf("First Close() failed: %v", err) + } + + // Second close should also succeed (idempotent) and not return EBADF. + if err := w.Close(); err != nil { + t.Fatalf("Second Close() failed: %v", err) + } +}