Skip to content

Commit 8a89a2f

Browse files
nixprimegvisor-bot
authored andcommitted
vfs: call UnregisterFileAsyncHandler before FileDescriptionImpl.Release
The implementation of the former may depend on resources released by the latter. PiperOrigin-RevId: 787321427
1 parent d693cf4 commit 8a89a2f

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

pkg/sentry/vfs/file_description.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,18 +193,20 @@ func (fd *FileDescription) DecRef(ctx context.Context) {
193193
fd.impl.UnlockPOSIX(ctx, fd, lock.LockRange{0, lock.LockEOF})
194194
}
195195

196-
// Release implementation resources.
197-
fd.impl.Release(ctx)
198-
if fd.writable {
199-
fd.vd.mount.EndWrite()
200-
}
201-
fd.vd.DecRef(ctx)
196+
// Clean up O_ASYNC state.
202197
fd.flagsMu.Lock()
203198
if fd.statusFlags.RacyLoad()&linux.O_ASYNC != 0 && fd.asyncHandler != nil {
204199
fd.impl.UnregisterFileAsyncHandler(fd)
205200
}
206201
fd.asyncHandler = nil
207202
fd.flagsMu.Unlock()
203+
204+
// Release implementation resources.
205+
fd.impl.Release(ctx)
206+
if fd.writable {
207+
fd.vd.mount.EndWrite()
208+
}
209+
fd.vd.DecRef(ctx)
208210
})
209211
}
210212

test/syscalls/linux/fcntl.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,6 +1767,24 @@ TEST(FcntlTest, SetFlSetOwnSetSigDoNotRace) {
17671767
}
17681768
}
17691769

1770+
TEST(FcntlTest, RegularFileOAsync) {
1771+
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
1772+
FileDescriptor fd =
1773+
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
1774+
1775+
// Linux allows, but silently ignores, fcntl(F_SETFL, O_ASYNC) on regular
1776+
// files (more precisely, files for which file_operations::fasync == nil). As
1777+
// of this writing, gVisor allows and implements O_ASYNC on all files. (It's
1778+
// not clear that this is safe from a lock ordering perspective, so this may
1779+
// be fixed in the future.) For now, just test that enabling O_ASYNC succeeds
1780+
// and that closing the file does not panic the sentry (b/430624153).
1781+
ASSERT_THAT(fcntl(fd.get(), F_SETOWN, getpid()), SyscallSucceeds());
1782+
ASSERT_THAT(fcntl(fd.get(), F_SETSIG, SIGIO), SyscallSucceeds());
1783+
int flags;
1784+
ASSERT_THAT(flags = fcntl(fd.get(), F_GETFL), SyscallSucceeds());
1785+
ASSERT_THAT(fcntl(fd.get(), F_SETFL, flags | O_ASYNC), SyscallSucceeds());
1786+
}
1787+
17701788
TEST_F(FcntlLockTest, GetLockOnNothing) {
17711789
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
17721790
FileDescriptor fd =

0 commit comments

Comments
 (0)