diff --git a/pkg/sentry/syscalls/linux/linux64.go b/pkg/sentry/syscalls/linux/linux64.go index 62d1e8f8b..4747117b8 100644 --- a/pkg/sentry/syscalls/linux/linux64.go +++ b/pkg/sentry/syscalls/linux/linux64.go @@ -272,7 +272,7 @@ var AMD64 = &kernel.SyscallTable{ 217: syscalls.Supported("getdents64", Getdents64), 218: syscalls.Supported("set_tid_address", SetTidAddress), 219: syscalls.Supported("restart_syscall", RestartSyscall), - 220: syscalls.PartiallySupported("semtimedop", Semtimedop, "A non-zero timeout argument isn't supported.", []string{"gvisor.dev/issue/137"}), + 220: syscalls.Supported("semtimedop", Semtimedop), 221: syscalls.PartiallySupported("fadvise64", Fadvise64, "Not all options are supported.", nil), 222: syscalls.Supported("timer_create", TimerCreate), 223: syscalls.Supported("timer_settime", TimerSettime), @@ -620,7 +620,7 @@ var ARM64 = &kernel.SyscallTable{ 189: syscalls.ErrorWithEvent("msgsnd", syserror.ENOSYS, "", []string{"gvisor.dev/issue/135"}), // TODO(b/29354921) 190: syscalls.Supported("semget", Semget), 191: syscalls.PartiallySupported("semctl", Semctl, "Options SEM_STAT_ANY not supported.", nil), - 192: syscalls.PartiallySupported("semtimedop", Semtimedop, "A non-zero timeout argument isn't supported.", []string{"gvisor.dev/issue/137"}), + 192: syscalls.Supported("semtimedop", Semtimedop), 193: syscalls.PartiallySupported("semop", Semop, "Option SEM_UNDO not supported.", nil), 194: syscalls.PartiallySupported("shmget", Shmget, "Option SHM_HUGETLB is not supported.", nil), 195: syscalls.PartiallySupported("shmctl", Shmctl, "Options SHM_LOCK, SHM_UNLOCK are not supported.", nil), diff --git a/pkg/sentry/syscalls/linux/sys_sem.go b/pkg/sentry/syscalls/linux/sys_sem.go index d324461a3..55287f147 100644 --- a/pkg/sentry/syscalls/linux/sys_sem.go +++ b/pkg/sentry/syscalls/linux/sys_sem.go @@ -16,6 +16,7 @@ package linux import ( "math" + "time" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/marshal/primitive" @@ -50,24 +51,15 @@ func Semget(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal // Semtimedop handles: semop(int semid, struct sembuf *sops, size_t nsops, const struct timespec *timeout) func Semtimedop(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { - // TODO(gvisor.dev/issue/137): A non-zero timeout isn't supported. - if args[3].Pointer() != 0 { - return 0, nil, syserror.ENOSYS + // If the timeout argument is NULL, then semtimedop() behaves exactly like semop(). + if args[3].Pointer() == 0 { + return Semop(t, args) } - return Semop(t, args) -} -// Semop handles: semop(int semid, struct sembuf *sops, size_t nsops) -func Semop(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { id := args[0].Int() sembufAddr := args[1].Pointer() nsops := args[2].SizeT() - - r := t.IPCNamespace().SemaphoreRegistry() - set := r.FindByID(id) - if set == nil { - return 0, nil, syserror.EINVAL - } + timespecAddr := args[3].Pointer() if nsops <= 0 { return 0, nil, syserror.EINVAL } @@ -80,17 +72,59 @@ func Semop(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall return 0, nil, err } + var timeout linux.Timespec + if _, err := timeout.CopyIn(t, timespecAddr); err != nil { + return 0, nil, err + } + if timeout.Sec < 0 || timeout.Nsec < 0 || timeout.Nsec >= 1e9 { + return 0, nil, syserror.EINVAL + } + + if err := semTimedOp(t, id, ops, true, timeout.ToDuration()); err != nil { + if err == syserror.ETIMEDOUT { + return 0, nil, syserror.EAGAIN + } + return 0, nil, err + } + return 0, nil, nil +} + +// Semop handles: semop(int semid, struct sembuf *sops, size_t nsops) +func Semop(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { + id := args[0].Int() + sembufAddr := args[1].Pointer() + nsops := args[2].SizeT() + + if nsops <= 0 { + return 0, nil, syserror.EINVAL + } + if nsops > opsMax { + return 0, nil, syserror.E2BIG + } + + ops := make([]linux.Sembuf, nsops) + if _, err := linux.CopySembufSliceIn(t, sembufAddr, ops); err != nil { + return 0, nil, err + } + return 0, nil, semTimedOp(t, id, ops, false, time.Second) +} + +func semTimedOp(t *kernel.Task, id int32, ops []linux.Sembuf, haveTimeout bool, timeout time.Duration) error { + set := t.IPCNamespace().SemaphoreRegistry().FindByID(id) + + if set == nil { + return syserror.EINVAL + } creds := auth.CredentialsFromContext(t) pid := t.Kernel().GlobalInit().PIDNamespace().IDOfThreadGroup(t.ThreadGroup()) for { ch, num, err := set.ExecuteOps(t, ops, creds, int32(pid)) if ch == nil || err != nil { - // We're done (either on success or a failure). - return 0, nil, err + return err } - if err = t.Block(ch); err != nil { + if _, err = t.BlockWithTimeout(ch, haveTimeout, timeout); err != nil { set.AbortWait(num, ch) - return 0, nil, err + return err } } } diff --git a/test/syscalls/linux/semaphore.cc b/test/syscalls/linux/semaphore.cc index 0530fce44..b8f6ced30 100644 --- a/test/syscalls/linux/semaphore.cc +++ b/test/syscalls/linux/semaphore.cc @@ -116,6 +116,41 @@ TEST(SemaphoreTest, SemOpSingleNoBlock) { ASSERT_THAT(semop(sem.get(), nullptr, 0), SyscallFailsWithErrno(EINVAL)); } +// Tests simple timed operations that shouldn't block in a single-thread. +TEST(SemaphoreTest, SemTimedOpSingleNoBlock) { + AutoSem sem(semget(IPC_PRIVATE, 1, 0600 | IPC_CREAT)); + ASSERT_THAT(sem.get(), SyscallSucceeds()); + + struct sembuf buf = {}; + buf.sem_op = 1; + struct timespec timeout = {}; + // 50 milliseconds. + timeout.tv_nsec = 5e7; + ASSERT_THAT(semtimedop(sem.get(), &buf, 1, &timeout), SyscallSucceeds()); + + buf.sem_op = -1; + EXPECT_THAT(semtimedop(sem.get(), &buf, 1, &timeout), SyscallSucceeds()); + + buf.sem_op = 0; + EXPECT_THAT(semtimedop(sem.get(), &buf, 1, &timeout), SyscallSucceeds()); + + // Error cases with invalid values. + EXPECT_THAT(semtimedop(sem.get() + 1, &buf, 1, &timeout), + SyscallFailsWithErrno(EINVAL)); + + buf.sem_num = 1; + EXPECT_THAT(semtimedop(sem.get(), &buf, 1, &timeout), + SyscallFailsWithErrno(EFBIG)); + buf.sem_num = 0; + + EXPECT_THAT(semtimedop(sem.get(), nullptr, 0, &timeout), + SyscallFailsWithErrno(EINVAL)); + + timeout.tv_nsec = 1e9; + EXPECT_THAT(semtimedop(sem.get(), &buf, 0, &timeout), + SyscallFailsWithErrno(EINVAL)); +} + // Tests multiple operations that shouldn't block in a single-thread. TEST(SemaphoreTest, SemOpMultiNoBlock) { AutoSem sem(semget(IPC_PRIVATE, 4, 0600 | IPC_CREAT)); @@ -184,6 +219,29 @@ TEST(SemaphoreTest, SemOpBlock) { blocked.store(0); } +// Makes a best effort attempt to ensure that operation would be timeout when +// being blocked. +TEST(SemaphoreTest, SemTimedOpBlock) { + AutoSem sem(semget(IPC_PRIVATE, 1, 0600 | IPC_CREAT)); + ASSERT_THAT(sem.get(), SyscallSucceeds()); + + ScopedThread th([&sem] { + absl::SleepFor(absl::Milliseconds(100)); + + struct sembuf buf = {}; + buf.sem_op = 1; + ASSERT_THAT(RetryEINTR(semop)(sem.get(), &buf, 1), SyscallSucceeds()); + }); + + struct sembuf buf = {}; + buf.sem_op = -1; + struct timespec timeout = {}; + timeout.tv_nsec = 5e7; + // semtimedop reaches the time limit, it fails with errno EAGAIN. + ASSERT_THAT(RetryEINTR(semtimedop)(sem.get(), &buf, 1, &timeout), + SyscallFailsWithErrno(EAGAIN)); +} + // Tests that IPC_NOWAIT returns with no wait. TEST(SemaphoreTest, SemOpNoBlock) { AutoSem sem(semget(IPC_PRIVATE, 1, 0600 | IPC_CREAT));