Implement OFD POSIX locks.

This change also switches lock EOF to be MaxInt instead of MaxUint,
since the lock length can be a negative value.

Fixes #5264

PiperOrigin-RevId: 513571374
This commit is contained in:
Lucas Manning
2023-03-02 10:14:34 -08:00
committed by gVisor bot
parent 1e0fa752a2
commit 56d6af8a6f
7 changed files with 258 additions and 35 deletions
+3
View File
@@ -30,6 +30,9 @@ const (
F_GETSIG = 11
F_SETOWN_EX = 15
F_GETOWN_EX = 16
F_OFD_GETLK = 36
F_OFD_SETLK = 37
F_OFD_SETLKW = 38
F_DUPFD_CLOEXEC = 1024 + 6
F_SETPIPE_SZ = 1024 + 7
F_GETPIPE_SZ = 1024 + 8
+17 -21
View File
@@ -84,17 +84,16 @@ const (
//
// A BSD-style full file lock can be represented as a regional file lock from
// offset 0 to LockEOF.
const LockEOF = math.MaxUint64
const LockEOF = math.MaxInt64
// OwnerInfo describes the owner of a lock.
//
// TODO(gvisor.dev/issue/5264): We may need to add other fields in the future
// (e.g., Linux's file_lock.fl_flags to support open file-descriptor locks).
//
// +stateify savable
type OwnerInfo struct {
// PID is the process ID of the lock owner.
PID int32
// OFD is whether this is an open file descriptor lock.
OFD bool
}
// Lock is a regional file lock. It consists of either a single writer
@@ -139,7 +138,7 @@ type Locks struct {
// LockRegion attempts to acquire a typed lock for the uid on a region of a
// file. Returns nil if successful in locking the region, otherwise an
// appropriate error is returned.
func (l *Locks) LockRegion(ctx context.Context, uid UniqueID, ownerPID int32, t LockType, r LockRange, block bool) error {
func (l *Locks) LockRegion(ctx context.Context, uid UniqueID, ownerPID int32, t LockType, r LockRange, ofd bool, block bool) error {
l.mu.Lock()
defer l.mu.Unlock()
for {
@@ -147,7 +146,7 @@ func (l *Locks) LockRegion(ctx context.Context, uid UniqueID, ownerPID int32, t
// Blocking locks must run in a loop because we'll be woken up whenever an unlock event
// happens for this lock. We will then attempt to take the lock again and if it fails
// continue blocking.
err := l.locks.lock(uid, ownerPID, t, r)
err := l.locks.lock(uid, ownerPID, t, r, ofd)
if err == linuxerr.ErrWouldBlock && block {
// Note: we release the lock in EventRegister below, in
// order to avoid a possible race.
@@ -197,14 +196,14 @@ func (l *Locks) UnlockRegion(uid UniqueID, r LockRange) {
// makeLock returns a new typed Lock that has either uid as its only reader
// or uid as its only writer.
func makeLock(uid UniqueID, ownerPID int32, t LockType) Lock {
func makeLock(uid UniqueID, ownerPID int32, t LockType, ofd bool) Lock {
value := Lock{Readers: make(map[UniqueID]OwnerInfo)}
switch t {
case ReadLock:
value.Readers[uid] = OwnerInfo{PID: ownerPID}
value.Readers[uid] = OwnerInfo{PID: ownerPID, OFD: ofd}
case WriteLock:
value.Writer = uid
value.WriterInfo = OwnerInfo{PID: ownerPID}
value.WriterInfo = OwnerInfo{PID: ownerPID, OFD: ofd}
default:
panic(fmt.Sprintf("makeLock: invalid lock type %d", t))
}
@@ -222,7 +221,7 @@ func (l Lock) isHeld(uid UniqueID) bool {
// lock sets uid as a holder of a typed lock on Lock.
//
// Preconditions: canLock is true for the range containing this Lock.
func (l *Lock) lock(uid UniqueID, ownerPID int32, t LockType) {
func (l *Lock) lock(uid UniqueID, ownerPID int32, t LockType, ofd bool) {
switch t {
case ReadLock:
// If we are already a reader, then this is a no-op.
@@ -240,7 +239,7 @@ func (l *Lock) lock(uid UniqueID, ownerPID int32, t LockType) {
// Ensure that there is no longer a writer.
l.Writer = nil
}
l.Readers[uid] = OwnerInfo{PID: ownerPID}
l.Readers[uid] = OwnerInfo{PID: ownerPID, OFD: ofd}
return
case WriteLock:
// If we are already the writer, then this is a no-op.
@@ -261,7 +260,7 @@ func (l *Lock) lock(uid UniqueID, ownerPID int32, t LockType) {
// Ensure that there is only a writer.
l.Readers = make(map[UniqueID]OwnerInfo)
l.Writer = uid
l.WriterInfo = OwnerInfo{PID: ownerPID}
l.WriterInfo = OwnerInfo{PID: ownerPID, OFD: ofd}
default:
panic(fmt.Sprintf("lock: invalid lock type %d", t))
}
@@ -331,7 +330,7 @@ func (l *Lock) isOnlyReader(uid UniqueID) bool {
// LockRange. Otherwise, linuxerr.ErrWouldBlock is returned.
//
// Preconditions: r.Start <= r.End (will panic otherwise).
func (l *LockSet) lock(uid UniqueID, ownerPID int32, t LockType, r LockRange) error {
func (l *LockSet) lock(uid UniqueID, ownerPID int32, t LockType, r LockRange, ofd bool) error {
if r.Start > r.End {
panic(fmt.Sprintf("lock: r.Start %d > r.End %d", r.Start, r.End))
}
@@ -353,7 +352,7 @@ func (l *LockSet) lock(uid UniqueID, ownerPID int32, t LockType, r LockRange) er
seg, gap := l.Find(r.Start)
if gap.Ok() {
// Fill in the gap and get the next segment to modify.
seg = l.Insert(gap, gap.Range().Intersect(r), makeLock(uid, ownerPID, t)).NextSegment()
seg = l.Insert(gap, gap.Range().Intersect(r), makeLock(uid, ownerPID, t, ofd)).NextSegment()
} else if seg.Start() < r.Start {
// Get our first segment to modify.
_, seg = l.Split(seg, r.Start)
@@ -367,12 +366,12 @@ func (l *LockSet) lock(uid UniqueID, ownerPID int32, t LockType, r LockRange) er
// Set the lock on the segment. This is guaranteed to
// always be safe, given canLock above.
value := seg.ValuePtr()
value.lock(uid, ownerPID, t)
value.lock(uid, ownerPID, t, ofd)
// Fill subsequent gaps.
gap = seg.NextGap()
if gr := gap.Range().Intersect(r); gr.Length() > 0 {
seg = l.Insert(gap, gr, makeLock(uid, ownerPID, t)).NextSegment()
seg = l.Insert(gap, gr, makeLock(uid, ownerPID, t, ofd)).NextSegment()
} else {
seg = gap.NextSegment()
}
@@ -496,10 +495,7 @@ func ComputeRange(start, length, offset int64) (LockRange, error) {
// Note that the PID returned in the flock structure is relative to the root PID
// namespace. It needs to be converted to the caller's PID namespace before
// returning to userspace.
//
// TODO(gvisor.dev/issue/5264): we don't support OFD locks through fcntl, which
// would return a struct with pid = -1.
func (l *Locks) TestRegion(ctx context.Context, uid UniqueID, t LockType, r LockRange) linux.Flock {
func (l *Locks) TestRegion(ctx context.Context, uid UniqueID, t LockType, r LockRange, ofd bool) linux.Flock {
f := linux.Flock{Type: linux.F_UNLCK}
switch t {
case ReadLock:
@@ -517,7 +513,7 @@ func (l *Locks) TestRegion(ctx context.Context, uid UniqueID, t LockType, r Lock
l.testRegion(r, func(lock Lock, start, length uint64) bool {
if lock.Writer == nil {
for k, v := range lock.Readers {
if k != uid {
if k != uid && v.OFD == ofd {
// Stop at the first conflict detected.
f.Type = linux.F_RDLCK
f.PID = v.PID
+1 -1
View File
@@ -662,7 +662,7 @@ func TestSetLock(t *testing.T) {
l := fill(test.before)
r := LockRange{Start: test.start, End: test.end}
err := l.lock(test.uid, 0 /* ownerPID */, test.lockType, r)
err := l.lock(test.uid, 0 /* ownerPID */, test.lockType, r, false)
var got []entry
for seg := l.FirstSegment(); seg.Ok(); seg = seg.NextSegment() {
got = append(got, entry{
+32 -10
View File
@@ -681,11 +681,17 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall
err := tmpfs.AddSeals(file, args[2].Uint())
return 0, nil, err
case linux.F_SETLK:
return 0, nil, posixLock(t, args, file, false /* block */)
return 0, nil, posixLock(t, args, file, false /* ofd */, false /* block */)
case linux.F_SETLKW:
return 0, nil, posixLock(t, args, file, true /* block */)
return 0, nil, posixLock(t, args, file, false /* ofd */, true /* block */)
case linux.F_GETLK:
return 0, nil, posixTestLock(t, args, file)
return 0, nil, posixTestLock(t, args, file, false /* ofd */)
case linux.F_OFD_SETLK:
return 0, nil, posixLock(t, args, file, true /* ofd */, false /* block */)
case linux.F_OFD_SETLKW:
return 0, nil, posixLock(t, args, file, true /* ofd */, true /* block */)
case linux.F_OFD_GETLK:
return 0, nil, posixTestLock(t, args, file, true /* ofd */)
case linux.F_GETSIG:
a := file.AsyncHandler()
if a == nil {
@@ -779,7 +785,7 @@ func setAsyncOwner(t *kernel.Task, fd int, file *vfs.FileDescription, ownerType,
}
}
func posixTestLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescription) error {
func posixTestLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescription, ofd bool) error {
// Copy in the lock request.
flockAddr := args[2].Pointer()
var flock linux.Flock
@@ -799,12 +805,18 @@ func posixTestLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDes
if err != nil {
return err
}
uid := lock.UniqueID(t.FDTable())
if ofd {
uid = lock.UniqueID(file)
}
newFlock, err := file.TestPOSIX(t, t.FDTable(), typ, r)
newFlock, err := file.TestPOSIX(t, uid, typ, r)
if err != nil {
return err
}
newFlock.PID = translatePID(t.PIDNamespace().Root(), t.PIDNamespace(), newFlock.PID)
if !ofd {
newFlock.PID = translatePID(t.PIDNamespace().Root(), t.PIDNamespace(), newFlock.PID)
}
if _, err = newFlock.CopyOut(t, flockAddr); err != nil {
return err
}
@@ -821,13 +833,23 @@ func translatePID(old, new *kernel.PIDNamespace, pid int32) int32 {
return int32(new.IDOfTask(old.TaskWithID(kernel.ThreadID(pid))))
}
func posixLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescription, block bool) error {
func posixLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescription, ofd bool, block bool) error {
// Copy in the lock request.
flockAddr := args[2].Pointer()
var flock linux.Flock
if _, err := flock.CopyIn(t, flockAddr); err != nil {
return err
}
if ofd && flock.PID != 0 {
return linuxerr.EINVAL
}
uid := lock.UniqueID(t.FDTable())
pid := int32(t.TGIDInRoot())
if ofd {
uid = lock.UniqueID(file)
pid = -1
}
r, err := file.ComputeLockRange(t, uint64(flock.Start), uint64(flock.Len), flock.Whence)
if err != nil {
@@ -839,16 +861,16 @@ func posixLock(t *kernel.Task, args arch.SyscallArguments, file *vfs.FileDescrip
if !file.IsReadable() {
return linuxerr.EBADF
}
return file.LockPOSIX(t, t.FDTable(), int32(t.TGIDInRoot()), lock.ReadLock, r, block)
return file.LockPOSIX(t, uid, pid, lock.ReadLock, r, block)
case linux.F_WRLCK:
if !file.IsWritable() {
return linuxerr.EBADF
}
return file.LockPOSIX(t, t.FDTable(), int32(t.TGIDInRoot()), lock.WriteLock, r, block)
return file.LockPOSIX(t, uid, pid, lock.WriteLock, r, block)
case linux.F_UNLCK:
return file.UnlockPOSIX(t, t.FDTable(), r)
return file.UnlockPOSIX(t, uid, r)
default:
return linuxerr.EINVAL
+5
View File
@@ -188,6 +188,11 @@ func (fd *FileDescription) DecRef(ctx context.Context) {
fd.impl.UnlockBSD(context.Background(), fd)
}
// Unlock any OFD locks.
if fd.impl.SupportsLocks() {
fd.impl.UnlockPOSIX(ctx, fd, lock.LockRange{0, lock.LockEOF})
}
// Release implementation resources.
fd.impl.Release(ctx)
if fd.writable {
+5 -3
View File
@@ -40,7 +40,7 @@ type FileLocks struct {
// LockBSD tries to acquire a BSD-style lock on the entire file.
func (fl *FileLocks) LockBSD(ctx context.Context, uid fslock.UniqueID, ownerID int32, t fslock.LockType, block bool) error {
if err := fl.bsd.LockRegion(ctx, uid, ownerID, t, fslock.LockRange{0, fslock.LockEOF}, block); err == nil || err == linuxerr.ErrWouldBlock {
if err := fl.bsd.LockRegion(ctx, uid, ownerID, t, fslock.LockRange{0, fslock.LockEOF}, false, block); err == nil || err == linuxerr.ErrWouldBlock {
return err
}
return linuxerr.ERESTARTSYS
@@ -56,7 +56,8 @@ func (fl *FileLocks) UnlockBSD(uid fslock.UniqueID) {
// LockPOSIX tries to acquire a POSIX-style lock on a file region.
func (fl *FileLocks) LockPOSIX(ctx context.Context, uid fslock.UniqueID, ownerPID int32, t fslock.LockType, r fslock.LockRange, block bool) error {
if err := fl.posix.LockRegion(ctx, uid, ownerPID, t, r, block); err == nil || err == linuxerr.ErrWouldBlock {
_, ofd := uid.(*FileDescription)
if err := fl.posix.LockRegion(ctx, uid, ownerPID, t, r, ofd, block); err == nil || err == linuxerr.ErrWouldBlock {
return err
}
return linuxerr.ERESTARTSYS
@@ -73,5 +74,6 @@ func (fl *FileLocks) UnlockPOSIX(ctx context.Context, uid fslock.UniqueID, r fsl
// TestPOSIX returns information about whether the specified lock can be held, in the style of the F_GETLK fcntl.
func (fl *FileLocks) TestPOSIX(ctx context.Context, uid fslock.UniqueID, t fslock.LockType, r fslock.LockRange) (linux.Flock, error) {
return fl.posix.TestRegion(ctx, uid, t, r), nil
_, ofd := uid.(*FileDescription)
return fl.posix.TestRegion(ctx, uid, t, r, ofd), nil
}
+195
View File
@@ -22,12 +22,14 @@
#include <unistd.h>
#include <atomic>
#include <cerrno>
#include <deque>
#include <iostream>
#include <list>
#include <string>
#include <vector>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/base/macros.h"
#include "absl/base/port.h"
@@ -1954,6 +1956,199 @@ TEST_F(FcntlLockTest, GetLockRespectsPIDNamespace) {
ASSERT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0);
}
TEST_F(FcntlLockTest, TestOFDBasicLock) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
struct flock fl = {
.l_type = F_WRLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
FileDescriptor fd2 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
// Locking from a different file descriptor should fail.
ASSERT_THAT(fcntl(fd2.get(), F_OFD_SETLK, &fl),
SyscallFailsWithErrno(EAGAIN));
fl.l_type = F_UNLCK;
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
fl.l_type = F_WRLCK;
ASSERT_THAT(fcntl(fd2.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
}
TEST_F(FcntlLockTest, TestOFDLockNonZeroPidFails) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
struct flock fl = {
.l_type = F_WRLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 1,
};
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl),
SyscallFailsWithErrno(EINVAL));
}
TEST_F(FcntlLockTest, TestOFDNoUnlockOnClose) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct flock fl = {
.l_type = F_RDLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
FileDescriptor fd2 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
ASSERT_THAT(fcntl(fd2.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
FileDescriptor fd3 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
// Close should not release all locks, just the one associated with the closed
// file descriptor.
ASSERT_THAT(close(fd1.release()), SyscallSucceeds());
fl.l_type = F_WRLCK;
ASSERT_THAT(fcntl(fd3.get(), F_OFD_GETLK, &fl), SyscallSucceeds());
ASSERT_EQ(fl.l_type, F_RDLCK);
}
TEST_F(FcntlLockTest, TestOFDUnlocksOnLastClose) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct flock fl = {
.l_type = F_RDLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
FileDescriptor fd2 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
ASSERT_THAT(fcntl(fd2.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
ASSERT_THAT(close(fd1.release()), SyscallSucceeds());
ASSERT_THAT(close(fd2.release()), SyscallSucceeds());
FileDescriptor fd3 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
fl.l_type = F_WRLCK;
ASSERT_THAT(fcntl(fd3.get(), F_OFD_GETLK, &fl), SyscallSucceeds());
ASSERT_EQ(fl.l_type, F_UNLCK);
}
TEST_F(FcntlLockTest, TestOFDInheritsLockAfterDup) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct flock fl = {
.l_type = F_WRLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
FileDescriptor duped = ASSERT_NO_ERRNO_AND_VALUE(fd1.Dup());
ASSERT_THAT(close(fd1.release()), SyscallSucceeds());
FileDescriptor fd2 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd2.get(), F_OFD_SETLK, &fl),
SyscallFailsWithErrno(EAGAIN));
}
TEST_F(FcntlLockTest, TestOFDLocksHoldAfterExec) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
// Setup two regional locks with different permissions.
struct flock fl0;
fl0.l_type = F_WRLCK;
fl0.l_whence = SEEK_SET;
fl0.l_start = 0;
fl0.l_len = 4096;
fl0.l_pid = 0;
struct flock fl1;
fl1.l_type = F_RDLCK;
fl1.l_whence = SEEK_SET;
fl1.l_start = 4096;
// Same as SetLockBadFd.
fl1.l_len = 0;
fl1.l_pid = 0;
// Set both region locks.
EXPECT_THAT(fcntl(fd.get(), F_OFD_SETLK, &fl0), SyscallSucceeds());
EXPECT_THAT(fcntl(fd.get(), F_OFD_SETLK, &fl1), SyscallSucceeds());
// Another process should fail to take a read lock on the entire file
// due to the regional write lock.
pid_t child_pid = 0;
auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(
SubprocessLock(file.path(), false /* write lock */,
false /* nonblocking */, false /* no eintr retry */,
nullptr /* no socket fd */, 0, 0, &child_pid));
int status = 0;
ASSERT_THAT(RetryEINTR(waitpid)(child_pid, &status, 0), SyscallSucceeds());
EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == EAGAIN)
<< "Exited with code: " << status;
}
TEST_F(FcntlLockTest, TestOFDGetLkReturnsNegPID) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct flock fl = {
.l_type = F_WRLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
FileDescriptor fd2 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd2.get(), F_OFD_GETLK, &fl), SyscallSucceeds());
ASSERT_EQ(fl.l_pid, -1);
}
TEST_F(FcntlLockTest, TestOFDCanUpgradeLock) {
auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct flock fl = {
.l_type = F_RDLCK,
.l_whence = SEEK_SET,
.l_start = 0,
.l_len = 0,
.l_pid = 0,
};
FileDescriptor fd1 =
ASSERT_NO_ERRNO_AND_VALUE(Open(file.path(), O_RDWR, 0666));
ASSERT_THAT(fcntl(fd1.get(), F_OFD_SETLK, &fl), SyscallSucceeds());
fl.l_type = F_WRLCK;
ASSERT_THAT(fcntl(fd1.get(), F_OFD_GETLK, &fl), SyscallSucceeds());
ASSERT_EQ(fl.l_type, F_UNLCK);
}
} // namespace
} // namespace testing