netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Call FileDescription.OnClose() for newfd being replaced in dup2 and dup3.

Browse Source 
dup(2) man page specifies:
       If the file descriptor newfd was previously open, it is closed
       before being reused; the close is performed silently (i.e., any
       errors during the close are not reported by dup2()).

Even though we were DecRef-ing and hence releasing the replaced FD, we were
not calling OnClose(). Compare fs/file.c:do_dup2() -> filp_close(tofree), which
in turn calls filp_flush(). In gVisor, FileDescription.OnClose() analogously
does such flush operations.
in turn

PiperOrigin-RevId: 583147682
This commit is contained in:
Ayush Ranjan
2023-11-16 13:38:17 -08:00
committed by gVisor bot
parent 77b137ffd8
commit 980de72deb
5 changed files with 46 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/fdimport/fdimport.go
+6 -1
View File
@@ -93,9 +93,14 @@ func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth
hostFD.Release() // FD is transferred to host FD.
}
if err := fdTable.NewFDAt(ctx, int32(appFD), appFile, fdFlags[hostFD]); err != nil {
df, err := fdTable.NewFDAt(ctx, int32(appFD), appFile, fdFlags[hostFD])
if err != nil {
return nil, err
}
if df != nil {
df.DecRef(ctx)
return nil, fmt.Errorf("app FD %d displaced while importing FDs", appFD)
}
}
if ttyFile == nil {
return nil, nil
pkg/sentry/kernel/fd_table.go
+18 -22
View File
@@ -123,18 +123,14 @@ func (f *FDTable) loadDescriptorTable(m map[int32]descriptor) {
}
}
// drop drops the table reference.
func (f *FDTable) drop(ctx context.Context, file *vfs.FileDescription) {
// Release any POSIX lock possibly held by the FDTable.
// Release any POSIX lock possibly held by the FDTable.
func (f *FDTable) fileUnlock(ctx context.Context, file *vfs.FileDescription) {
if file.SupportsLocks() {
err := file.UnlockPOSIX(ctx, f, lock.LockRange{0, lock.LockEOF})
if err != nil && !linuxerr.Equals(linuxerr.ENOLCK, err) {
panic(fmt.Sprintf("UnlockPOSIX failed: %v", err))
}
}
// Drop the table's reference.
file.DecRef(ctx)
}
// NewFDTable allocates a new FDTable that may be used by tasks in k.
@@ -312,23 +308,24 @@ func (f *FDTable) NewFD(ctx context.Context, minFD int32, file *vfs.FileDescript
}
// NewFDAt sets the file reference for the given FD. If there is an existing
// file description for that FD, the table reference for that file description
// is dropped.
// file description for that FD, it is returned.
//
// N.B. Callers are required to use DecRef on the returned file when they are done.
//
// Precondition: file != nil.
func (f *FDTable) NewFDAt(ctx context.Context, fd int32, file *vfs.FileDescription, flags FDFlags) error {
func (f *FDTable) NewFDAt(ctx context.Context, fd int32, file *vfs.FileDescription, flags FDFlags) (*vfs.FileDescription, error) {
if fd < 0 {
// Don't accept negative FDs.
return unix.EBADF
return nil, unix.EBADF
}
if fd >= f.k.MaxFDLimit.Load() {
return unix.EMFILE
return nil, unix.EMFILE
}
// Check the limit for the provided file.
if limitSet := limits.FromContext(ctx); limitSet != nil {
if lim := limitSet.Get(limits.NumberOfFiles); lim.Cur != limits.Infinity && uint64(fd) >= lim.Cur {
return unix.EMFILE
return nil, unix.EMFILE
}
}
@@ -342,9 +339,10 @@ func (f *FDTable) NewFDAt(ctx context.Context, fd int32, file *vfs.FileDescripti
f.mu.Unlock()
if df != nil {
f.drop(ctx, df)
f.fileUnlock(ctx, df)
// Table's reference on df is transferred to caller, so don't DecRef.
}
return nil
return df, nil
}
// SetFlags sets the flags for the given file descriptor.
@@ -470,14 +468,13 @@ func (f *FDTable) Remove(ctx context.Context, fd int32) *vfs.FileDescription {
f.mu.Lock()
df := f.set(fd, nil, FDFlags{}) // Zap entry.
if df != nil {
// Add reference for caller.
df.IncRef()
f.fdBitmap.Remove(uint32(fd))
}
f.mu.Unlock()
if df != nil {
f.drop(ctx, df)
f.fileUnlock(ctx, df)
// Table's reference on df is transferred to caller, so don't DecRef.
}
return df
}
@@ -499,7 +496,8 @@ func (f *FDTable) RemoveIf(ctx context.Context, cond func(*vfs.FileDescription,
f.mu.Unlock()
for _, file := range files {
f.drop(ctx, file)
f.fileUnlock(ctx, file)
file.DecRef(ctx) // Drop the table's reference.
}
}
@@ -513,7 +511,6 @@ func (f *FDTable) RemoveNextInRange(ctx context.Context, startFd int32, endFd in
}
f.mu.Lock()
fdUint, err := f.fdBitmap.FirstOne(uint32(startFd))
fd := int32(fdUint)
if err != nil || fd > endFd {
@@ -522,14 +519,13 @@ func (f *FDTable) RemoveNextInRange(ctx context.Context, startFd int32, endFd in
}
df := f.set(fd, nil, FDFlags{}) // Zap entry.
if df != nil {
// Add reference for caller.
df.IncRef()
f.fdBitmap.Remove(uint32(fd))
}
f.mu.Unlock()
if df != nil {
f.drop(ctx, df)
f.fileUnlock(ctx, df)
// Table's reference on df is transferred to caller, so don't DecRef.
}
return fd, df
}
pkg/sentry/kernel/fd_table_test.go
+13 -7
View File
@@ -92,8 +92,10 @@ func TestFDTableMany(t *testing.T) {
t.Fatalf("fdTable.NewFDs(0, r) in full map: got nil, wanted error")
}
if err := fdTable.NewFDAt(ctx, 1, fd, FDFlags{}); err != nil {
if df, err := fdTable.NewFDAt(ctx, 1, fd, FDFlags{}); err != nil {
t.Fatalf("fdTable.NewFDAt(1, r, FDFlags{}): got %v, wanted nil", err)
} else if df != nil {
t.Fatalf("fdTable.NewFDAt(1, r, FDFlags{}) displaced FD")
}
i := int32(2)
@@ -159,11 +161,13 @@ func TestFDTable(t *testing.T) {
t.Fatalf("Added an FD to a resized map: got %v, want {1}", fds)
}
if err := fdTable.NewFDAt(ctx, 1, fd, FDFlags{}); err != nil {
if df, err := fdTable.NewFDAt(ctx, 1, fd, FDFlags{}); err != nil {
t.Fatalf("Replacing FD 1 via fdTable.NewFDAt(1, r, FDFlags{}): got %v, wanted nil", err)
} else if df != nil {
t.Fatalf("fdTable.NewFDAt(1, r, FDFlags{}) displaced FD")
}
if err := fdTable.NewFDAt(ctx, maxFD+1, fd, FDFlags{}); err == nil {
if _, err := fdTable.NewFDAt(ctx, maxFD+1, fd, FDFlags{}); err == nil {
t.Fatalf("Using an FD that was too large via fdTable.NewFDAt(%v, r, FDFlags{}): got nil, wanted an error", maxFD+1)
}
@@ -189,8 +193,10 @@ func TestFDTable(t *testing.T) {
func TestDescriptorFlags(t *testing.T) {
runTest(t, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, _ *limits.LimitSet) {
if err := fdTable.NewFDAt(ctx, 2, fd, FDFlags{CloseOnExec: true}); err != nil {
if df, err := fdTable.NewFDAt(ctx, 2, fd, FDFlags{CloseOnExec: true}); err != nil {
t.Fatalf("fdTable.NewFDAt(2, r, FDFlags{}): got %v, wanted nil", err)
} else if df != nil {
t.Fatalf("fdTable.NewFDAt(2, r, FDFlags{}) displaced FD")
}
newFile, flags := fdTable.Get(2)
@@ -231,7 +237,7 @@ func BenchmarkNewFDAt(b *testing.B) {
b.StartTimer() // Benchmark.
for i := 0; i < b.N; i++ {
err := fdTable.NewFDAt(ctx, int32(i%maxLimit), fd, FDFlags{})
_, err := fdTable.NewFDAt(ctx, int32(i%maxLimit), fd, FDFlags{})
if err != nil {
b.Fatalf("fdTable.NewFDAt: got %v, wanted nil", err)
}
@@ -244,7 +250,7 @@ func BenchmarkFork(b *testing.B) {
runTest(b, func(ctx context.Context, fdTable *FDTable, fd *vfs.FileDescription, limitSet *limits.LimitSet) {
for i := 0; i < maxFD; i++ {
err := fdTable.NewFDAt(ctx, int32(i), fd, FDFlags{})
_, err := fdTable.NewFDAt(ctx, int32(i), fd, FDFlags{})
if err != nil {
b.Fatalf("fdTable.NewFDs: got %v, wanted nil", err)
}
@@ -267,7 +273,7 @@ func BenchmarkCreateWithMaxFD(b *testing.B) {
for i := 0; i < b.N; i++ {
fdTable := new(FDTable)
fdTable.init()
err := fdTable.NewFDAt(ctx, maxLimit-1, fd, FDFlags{})
_, err := fdTable.NewFDAt(ctx, maxLimit-1, fd, FDFlags{})
if err != nil {
b.Fatalf("fdTable.NewFDs: got %v, wanted nil", err)
}
pkg/sentry/kernel/task.go
+1 -1
View File
@@ -774,7 +774,7 @@ func (t *Task) NewFDFrom(minFD int32, file *vfs.FileDescription, flags FDFlags)
// This automatically passes the task as the context.
//
// Precondition: same as FDTable.
func (t *Task) NewFDAt(fd int32, file *vfs.FileDescription, flags FDFlags) error {
func (t *Task) NewFDAt(fd int32, file *vfs.FileDescription, flags FDFlags) (*vfs.FileDescription, error) {
return t.fdTable.NewFDAt(t, fd, file, flags)
}
pkg/sentry/syscalls/linux/sys_file.go
+8 -1
View File
@@ -562,7 +562,7 @@ func dup3(t *kernel.Task, oldfd, newfd int32, flags uint32) (uintptr, *kernel.Sy
}
defer file.DecRef(t)
err := t.NewFDAt(newfd, file, kernel.FDFlags{
df, err := t.NewFDAt(newfd, file, kernel.FDFlags{
CloseOnExec: flags&linux.O_CLOEXEC != 0,
})
if linuxerr.Equals(linuxerr.EMFILE, err) {
@@ -571,6 +571,13 @@ func dup3(t *kernel.Task, oldfd, newfd int32, flags uint32) (uintptr, *kernel.Sy
if err != nil {
return 0, nil, err
}
if df != nil {
// "If the file descriptor newfd was previously open, it is closed
// before being reused; the close is performed silently (i.e., any
// errors during the close are not reported by dup2())." - dup(2)
_ = df.OnClose(t)
df.DecRef(t)
}
return uintptr(newfd), nil, nil
}