Port GETOWN, SETOWN fcntls to vfs2.

Also make some fixes to vfs1's F_SETOWN. The fcntl test now entirely passes
on vfs2.

Fixes #2920.

PiperOrigin-RevId: 318669529
This commit is contained in:
Dean Deng
2020-06-27 21:33:37 -07:00
committed by gVisor bot
parent 02d552d07c
commit e8f1a5c1f6
9 changed files with 308 additions and 27 deletions
+1 -1
View File
@@ -55,7 +55,7 @@ type Flock struct {
_ [4]byte
}
// Flags for F_SETOWN_EX and F_GETOWN_EX.
// Owner types for F_SETOWN_EX and F_GETOWN_EX.
const (
F_OWNER_TID = 0
F_OWNER_PID = 1
+1
View File
@@ -11,6 +11,7 @@ go_library(
"//pkg/sentry/fs",
"//pkg/sentry/kernel",
"//pkg/sentry/kernel/auth",
"//pkg/sentry/vfs",
"//pkg/sync",
"//pkg/waiter",
],
+7 -1
View File
@@ -20,15 +20,21 @@ import (
"gvisor.dev/gvisor/pkg/sentry/fs"
"gvisor.dev/gvisor/pkg/sentry/kernel"
"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
"gvisor.dev/gvisor/pkg/sentry/vfs"
"gvisor.dev/gvisor/pkg/sync"
"gvisor.dev/gvisor/pkg/waiter"
)
// New creates a new FileAsync.
// New creates a new fs.FileAsync.
func New() fs.FileAsync {
return &FileAsync{}
}
// NewVFS2 creates a new vfs.FileAsync.
func NewVFS2() vfs.FileAsync {
return &FileAsync{}
}
// FileAsync sends signals when the registered file is ready for IO.
//
// +stateify savable
+10 -5
View File
@@ -900,14 +900,20 @@ func fGetOwn(t *kernel.Task, file *fs.File) int32 {
//
// If who is positive, it represents a PID. If negative, it represents a PGID.
// If the PID or PGID is invalid, the owner is silently unset.
func fSetOwn(t *kernel.Task, file *fs.File, who int32) {
func fSetOwn(t *kernel.Task, file *fs.File, who int32) error {
a := file.Async(fasync.New).(*fasync.FileAsync)
if who < 0 {
// Check for overflow before flipping the sign.
if who-1 > who {
return syserror.EINVAL
}
pg := t.PIDNamespace().ProcessGroupWithID(kernel.ProcessGroupID(-who))
a.SetOwnerProcessGroup(t, pg)
} else {
tg := t.PIDNamespace().ThreadGroupWithID(kernel.ThreadID(who))
a.SetOwnerThreadGroup(t, tg)
}
tg := t.PIDNamespace().ThreadGroupWithID(kernel.ThreadID(who))
a.SetOwnerThreadGroup(t, tg)
return nil
}
// Fcntl implements linux syscall fcntl(2).
@@ -1042,8 +1048,7 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall
case linux.F_GETOWN:
return uintptr(fGetOwn(t, file)), nil, nil
case linux.F_SETOWN:
fSetOwn(t, file, args[2].Int())
return 0, nil, nil
return 0, nil, fSetOwn(t, file, args[2].Int())
case linux.F_GETOWN_EX:
addr := args[2].Pointer()
owner := fGetOwnEx(t, file)
+1
View File
@@ -54,6 +54,7 @@ go_library(
"//pkg/sentry/fsimpl/tmpfs",
"//pkg/sentry/kernel",
"//pkg/sentry/kernel/auth",
"//pkg/sentry/kernel/fasync",
"//pkg/sentry/kernel/pipe",
"//pkg/sentry/kernel/time",
"//pkg/sentry/limits",
+93
View File
@@ -20,6 +20,7 @@ import (
"gvisor.dev/gvisor/pkg/sentry/fs/lock"
"gvisor.dev/gvisor/pkg/sentry/fsimpl/tmpfs"
"gvisor.dev/gvisor/pkg/sentry/kernel"
"gvisor.dev/gvisor/pkg/sentry/kernel/fasync"
"gvisor.dev/gvisor/pkg/sentry/kernel/pipe"
slinux "gvisor.dev/gvisor/pkg/sentry/syscalls/linux"
"gvisor.dev/gvisor/pkg/sentry/vfs"
@@ -154,6 +155,47 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall
return 0, nil, err
}
return uintptr(n), nil, nil
case linux.F_GETOWN:
a := file.AsyncHandler()
if a == nil {
return 0, nil, nil
}
owner := getAsyncOwner(t, a.(*fasync.FileAsync))
if owner.Type == linux.F_OWNER_PGRP {
return uintptr(-owner.PID), nil, nil
}
return uintptr(owner.PID), nil, nil
case linux.F_SETOWN:
who := args[2].Int()
ownerType := int32(linux.F_OWNER_PID)
if who < 0 {
// Check for overflow before flipping the sign.
if who-1 > who {
return 0, nil, syserror.EINVAL
}
ownerType = linux.F_OWNER_PGRP
who = -who
}
a := file.SetAsyncHandler(fasync.NewVFS2).(*fasync.FileAsync)
return 0, nil, setAsyncOwner(t, a, ownerType, who)
case linux.F_GETOWN_EX:
a := file.AsyncHandler()
if a == nil {
return 0, nil, nil
}
addr := args[2].Pointer()
owner := getAsyncOwner(t, a.(*fasync.FileAsync))
_, err := t.CopyOut(addr, &owner)
return 0, nil, err
case linux.F_SETOWN_EX:
addr := args[2].Pointer()
var owner linux.FOwnerEx
n, err := t.CopyIn(addr, &owner)
if err != nil {
return 0, nil, err
}
a := file.SetAsyncHandler(fasync.NewVFS2).(*fasync.FileAsync)
return uintptr(n), nil, setAsyncOwner(t, a, owner.Type, owner.PID)
case linux.F_GETPIPE_SZ:
pipefile, ok := file.Impl().(*pipe.VFSPipeFD)
if !ok {
@@ -177,6 +219,57 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall
}
}
func getAsyncOwner(t *kernel.Task, a *fasync.FileAsync) linux.FOwnerEx {
ot, otg, opg := a.Owner()
switch {
case ot != nil:
return linux.FOwnerEx{
Type: linux.F_OWNER_TID,
PID: int32(t.PIDNamespace().IDOfTask(ot)),
}
case otg != nil:
return linux.FOwnerEx{
Type: linux.F_OWNER_PID,
PID: int32(t.PIDNamespace().IDOfThreadGroup(otg)),
}
case opg != nil:
return linux.FOwnerEx{
Type: linux.F_OWNER_PGRP,
PID: int32(t.PIDNamespace().IDOfProcessGroup(opg)),
}
default:
return linux.FOwnerEx{}
}
}
func setAsyncOwner(t *kernel.Task, a *fasync.FileAsync, ownerType, pid int32) error {
switch ownerType {
case linux.F_OWNER_TID:
task := t.PIDNamespace().TaskWithID(kernel.ThreadID(pid))
if task == nil {
return syserror.ESRCH
}
a.SetOwnerTask(t, task)
return nil
case linux.F_OWNER_PID:
tg := t.PIDNamespace().ThreadGroupWithID(kernel.ThreadID(pid))
if tg == nil {
return syserror.ESRCH
}
a.SetOwnerThreadGroup(t, tg)
return nil
case linux.F_OWNER_PGRP:
pg := t.PIDNamespace().ProcessGroupWithID(kernel.ProcessGroupID(pid))
if pg == nil {
return syserror.ESRCH
}
a.SetOwnerProcessGroup(t, pg)
return nil
default:
return syserror.EINVAL
}
}
func posixLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescription, cmd int32) error {
// Copy in the lock request.
flockAddr := args[2].Pointer()
+68 -6
View File
@@ -42,11 +42,20 @@ type FileDescription struct {
// operations.
refs int64
// flagsMu protects statusFlags and asyncHandler below.
flagsMu sync.Mutex
// statusFlags contains status flags, "initialized by open(2) and possibly
// modified by fcntl()" - fcntl(2). statusFlags is accessed using atomic
// memory operations.
// modified by fcntl()" - fcntl(2). statusFlags can be read using atomic
// memory operations when it does not need to be synchronized with an
// access to asyncHandler.
statusFlags uint32
// asyncHandler handles O_ASYNC signal generation. It is set with the
// F_SETOWN or F_SETOWN_EX fcntls. For asyncHandler to be used, O_ASYNC must
// also be set by fcntl(2).
asyncHandler FileAsync
// epolls is the set of epollInterests registered for this FileDescription.
// epolls is protected by epollMu.
epollMu sync.Mutex
@@ -193,6 +202,13 @@ func (fd *FileDescription) DecRef() {
fd.vd.mount.EndWrite()
}
fd.vd.DecRef()
fd.flagsMu.Lock()
// TODO(gvisor.dev/issue/1663): We may need to unregister during save, as we do in VFS1.
if fd.statusFlags&linux.O_ASYNC != 0 && fd.asyncHandler != nil {
fd.asyncHandler.Unregister(fd)
}
fd.asyncHandler = nil
fd.flagsMu.Unlock()
} else if refs < 0 {
panic("FileDescription.DecRef() called without holding a reference")
}
@@ -276,7 +292,18 @@ func (fd *FileDescription) SetStatusFlags(ctx context.Context, creds *auth.Crede
}
// TODO(jamieliu): FileDescriptionImpl.SetOAsync()?
const settableFlags = linux.O_APPEND | linux.O_ASYNC | linux.O_DIRECT | linux.O_NOATIME | linux.O_NONBLOCK
atomic.StoreUint32(&fd.statusFlags, (oldFlags&^settableFlags)|(flags&settableFlags))
fd.flagsMu.Lock()
if fd.asyncHandler != nil {
// Use fd.statusFlags instead of oldFlags, which may have become outdated,
// to avoid double registering/unregistering.
if fd.statusFlags&linux.O_ASYNC == 0 && flags&linux.O_ASYNC != 0 {
fd.asyncHandler.Register(fd)
} else if fd.statusFlags&linux.O_ASYNC != 0 && flags&linux.O_ASYNC == 0 {
fd.asyncHandler.Unregister(fd)
}
}
fd.statusFlags = (oldFlags &^ settableFlags) | (flags & settableFlags)
fd.flagsMu.Unlock()
return nil
}
@@ -533,17 +560,23 @@ func (fd *FileDescription) StatFS(ctx context.Context) (linux.Statfs, error) {
return fd.impl.StatFS(ctx)
}
// Readiness returns fd's I/O readiness.
// Readiness implements waiter.Waitable.Readiness.
//
// It returns fd's I/O readiness.
func (fd *FileDescription) Readiness(mask waiter.EventMask) waiter.EventMask {
return fd.impl.Readiness(mask)
}
// EventRegister registers e for I/O readiness events in mask.
// EventRegister implements waiter.Waitable.EventRegister.
//
// It registers e for I/O readiness events in mask.
func (fd *FileDescription) EventRegister(e *waiter.Entry, mask waiter.EventMask) {
fd.impl.EventRegister(e, mask)
}
// EventUnregister unregisters e for I/O readiness events.
// EventUnregister implements waiter.Waitable.EventUnregister.
//
// It unregisters e for I/O readiness events.
func (fd *FileDescription) EventUnregister(e *waiter.Entry) {
fd.impl.EventUnregister(e)
}
@@ -770,3 +803,32 @@ func (fd *FileDescription) LockPOSIX(ctx context.Context, uid lock.UniqueID, t l
func (fd *FileDescription) UnlockPOSIX(ctx context.Context, uid lock.UniqueID, start, end uint64, whence int16) error {
return fd.impl.UnlockPOSIX(ctx, uid, start, end, whence)
}
// A FileAsync sends signals to its owner when w is ready for IO. This is only
// implemented by pkg/sentry/fasync:FileAsync, but we unfortunately need this
// interface to avoid circular dependencies.
type FileAsync interface {
Register(w waiter.Waitable)
Unregister(w waiter.Waitable)
}
// AsyncHandler returns the FileAsync for fd.
func (fd *FileDescription) AsyncHandler() FileAsync {
fd.flagsMu.Lock()
defer fd.flagsMu.Unlock()
return fd.asyncHandler
}
// SetAsyncHandler sets fd.asyncHandler if it has not been set before and
// returns it.
func (fd *FileDescription) SetAsyncHandler(newHandler func() FileAsync) FileAsync {
fd.flagsMu.Lock()
defer fd.flagsMu.Unlock()
if fd.asyncHandler == nil {
fd.asyncHandler = newHandler()
if fd.statusFlags&linux.O_ASYNC != 0 {
fd.asyncHandler.Register(fd)
}
}
return fd.asyncHandler
}
+1
View File
@@ -805,6 +805,7 @@ cc_binary(
"//test/util:save_util",
"//test/util:temp_path",
"//test/util:test_util",
"//test/util:thread_util",
"//test/util:timer_util",
],
)
+126 -14
View File
@@ -37,6 +37,7 @@
#include "test/util/save_util.h"
#include "test/util/temp_path.h"
#include "test/util/test_util.h"
#include "test/util/thread_util.h"
#include "test/util/timer_util.h"
ABSL_FLAG(std::string, child_setlock_on, "",
@@ -953,15 +954,18 @@ TEST(FcntlTest, DupAfterO_ASYNC) {
EXPECT_EQ(after & O_ASYNC, O_ASYNC);
}
TEST(FcntlTest, GetOwn) {
TEST(FcntlTest, GetOwnNone) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
EXPECT_EQ(syscall(__NR_fcntl, s.get(), F_GETOWN), 0);
// Use the raw syscall because the glibc wrapper may convert F_{GET,SET}OWN
// into F_{GET,SET}OWN_EX.
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN),
SyscallSucceedsWithValue(0));
MaybeSave();
}
TEST(FcntlTest, GetOwnEx) {
TEST(FcntlTest, GetOwnExNone) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
@@ -970,6 +974,70 @@ TEST(FcntlTest, GetOwnEx) {
SyscallSucceedsWithValue(0));
}
TEST(FcntlTest, SetOwnInvalidPid) {
SKIP_IF(IsRunningWithVFS1());
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, 12345678),
SyscallFailsWithErrno(ESRCH));
}
TEST(FcntlTest, SetOwnInvalidPgrp) {
SKIP_IF(IsRunningWithVFS1());
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, -12345678),
SyscallFailsWithErrno(ESRCH));
}
TEST(FcntlTest, SetOwnPid) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
pid_t pid;
EXPECT_THAT(pid = getpid(), SyscallSucceeds());
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, pid), SyscallSucceeds());
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN),
SyscallSucceedsWithValue(pid));
MaybeSave();
}
TEST(FcntlTest, SetOwnPgrp) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
pid_t pgid;
EXPECT_THAT(pgid = getpgrp(), SyscallSucceeds());
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, -pgid), SyscallSucceeds());
// Verify with F_GETOWN_EX; using F_GETOWN on Linux may incorrectly treat the
// negative return value as an error, converting the return value to -1 and
// setting errno accordingly.
f_owner_ex got_owner = {};
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN_EX, &got_owner),
SyscallSucceedsWithValue(0));
EXPECT_EQ(got_owner.type, F_OWNER_PGRP);
EXPECT_EQ(got_owner.pid, pgid);
MaybeSave();
}
// F_SETOWN flips the sign of negative values, an operation that is guarded
// against overflow.
TEST(FcntlTest, SetOwnOverflow) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, INT_MIN),
SyscallFailsWithErrno(EINVAL));
}
TEST(FcntlTest, SetOwnExInvalidType) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
@@ -1027,7 +1095,8 @@ TEST(FcntlTest, SetOwnExTid) {
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN_EX, &owner),
SyscallSucceeds());
EXPECT_EQ(syscall(__NR_fcntl, s.get(), F_GETOWN), owner.pid);
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN),
SyscallSucceedsWithValue(owner.pid));
MaybeSave();
}
@@ -1042,7 +1111,8 @@ TEST(FcntlTest, SetOwnExPid) {
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN_EX, &owner),
SyscallSucceeds());
EXPECT_EQ(syscall(__NR_fcntl, s.get(), F_GETOWN), owner.pid);
EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN),
SyscallSucceedsWithValue(owner.pid));
MaybeSave();
}
@@ -1050,18 +1120,21 @@ TEST(FcntlTest, SetOwnExPgrp) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
f_owner_ex owner = {};
owner.type = F_OWNER_PGRP;
EXPECT_THAT(owner.pid = getpgrp(), SyscallSucceeds());
f_owner_ex set_owner = {};
set_owner.type = F_OWNER_PGRP;
EXPECT_THAT(set_owner.pid = getpgrp(), SyscallSucceeds());
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN_EX, &owner),
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN_EX, &set_owner),
SyscallSucceeds());
// NOTE(igudger): I don't understand why, but this is flaky on Linux.
// GetOwnExPgrp (below) does not have this issue.
SKIP_IF(!IsRunningOnGvisor());
EXPECT_EQ(syscall(__NR_fcntl, s.get(), F_GETOWN), -owner.pid);
// Verify with F_GETOWN_EX; using F_GETOWN on Linux may incorrectly treat the
// negative return value as an error, converting the return value to -1 and
// setting errno accordingly.
f_owner_ex got_owner = {};
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN_EX, &got_owner),
SyscallSucceedsWithValue(0));
EXPECT_EQ(got_owner.type, set_owner.type);
EXPECT_EQ(got_owner.pid, set_owner.pid);
MaybeSave();
}
@@ -1119,6 +1192,45 @@ TEST(FcntlTest, GetOwnExPgrp) {
EXPECT_EQ(got_owner.pid, set_owner.pid);
}
// Make sure that making multiple concurrent changes to async signal generation
// does not cause any race issues.
TEST(FcntlTest, SetFlSetOwnDoNotRace) {
FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE(
Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0));
pid_t pid;
EXPECT_THAT(pid = getpid(), SyscallSucceeds());
constexpr absl::Duration runtime = absl::Milliseconds(300);
auto setAsync = [&s, &runtime] {
for (auto start = absl::Now(); absl::Now() - start < runtime;) {
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETFL, O_ASYNC),
SyscallSucceeds());
sched_yield();
}
};
auto resetAsync = [&s, &runtime] {
for (auto start = absl::Now(); absl::Now() - start < runtime;) {
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETFL, 0), SyscallSucceeds());
sched_yield();
}
};
auto setOwn = [&s, &pid, &runtime] {
for (auto start = absl::Now(); absl::Now() - start < runtime;) {
ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, pid),
SyscallSucceeds());
sched_yield();
}
};
std::list<ScopedThread> threads;
for (int i = 0; i < 10; i++) {
threads.emplace_back(setAsync);
threads.emplace_back(resetAsync);
threads.emplace_back(setOwn);
}
}
} // namespace
} // namespace testing