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

Handle O_PATH correctly with fchmodat(2) and utimensat(2).

Browse Source 
- As an optimization, when path was empty and it was allowed to use empty path,
  setstatat() was invoking the FD-impl directly, instead of going through the
  FS-impl, in hopes that open FDs can perform these operations more quickly.
  However, this optimization breaks fchmodat(AT_EMPTY_PATH) for O_PATH FDs
  because O_PATH FD-impl rejects SetStat() operation. So disabled this
  optimization for O_PATH FDs.
- However, this fix revealed that we were now allowing utimes() operations on
  O_PATH FDs via utimensat(opathFD, path=NULL). Linux disallows this as
  mentioned in the patch. So fixed utimes codebase to emulate Linux accurately.

Fixes #9453

PiperOrigin-RevId: 571174417
This commit is contained in:
Ayush Ranjan
2023-10-05 17:09:36 -07:00
committed by gVisor bot
parent 4120841184
commit f3de61deed
3 changed files with 74 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/syscalls/linux/sys_file.go
+31 -48
View File
@@ -1152,10 +1152,11 @@ func setstatat(t *kernel.Task, dirfd int32, path fspath.Path, shouldAllowEmptyPa
if dirfile == nil {
return linuxerr.EBADF
}
if !path.HasComponents() {
// Use FileDescription.SetStat() instead of
// VirtualFilesystem.SetStatAt(), since the former may be able
// to use opened file state to expedite the SetStat.
if !path.HasComponents() && dirfile.StatusFlags()&linux.O_PATH == 0 {
// For empty path, use FileDescription.SetStat() instead of
// VirtualFilesystem.SetStatAt(), since the former may be able to use
// opened file state to expedite the SetStat. Skip this optimization
// for FDs with O_PATH, since the FD impl always returns EBADF.
err := dirfile.SetStat(t, *opts)
dirfile.DecRef(t)
return err
@@ -1381,11 +1382,6 @@ func Utime(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr,
pathAddr := args[0].Pointer()
timesAddr := args[1].Pointer()
path, err := copyInPath(t, pathAddr)
if err != nil {
return 0, nil, err
}
opts := vfs.SetStatOptions{
Stat: linux.Statx{
Mask: linux.STATX_ATIME | linux.STATX_MTIME,
@@ -1403,7 +1399,7 @@ func Utime(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr,
opts.Stat.Mtime.Sec = times.Modtime
}
return 0, nil, setstatat(t, linux.AT_FDCWD, path, disallowEmptyPath, followFinalSymlink, &opts)
return 0, nil, utimes(t, linux.AT_FDCWD, pathAddr, followFinalSymlink, &opts)
}
// Utimes implements Linux syscall utimes(2).
@@ -1411,17 +1407,12 @@ func Utimes(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr,
pathAddr := args[0].Pointer()
timesAddr := args[1].Pointer()
path, err := copyInPath(t, pathAddr)
if err != nil {
return 0, nil, err
}
var opts vfs.SetStatOptions
if err := populateSetStatOptionsForUtimes(t, timesAddr, &opts); err != nil {
return 0, nil, err
}
return 0, nil, setstatat(t, linux.AT_FDCWD, path, disallowEmptyPath, followFinalSymlink, &opts)
return 0, nil, utimes(t, linux.AT_FDCWD, pathAddr, followFinalSymlink, &opts)
}
// Futimesat implements Linux syscall futimesat(2).
@@ -1430,26 +1421,12 @@ func Futimesat(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintp
pathAddr := args[1].Pointer()
timesAddr := args[2].Pointer()
// "If filename is NULL and dfd refers to an open file, then operate on the
// file. Otherwise look up filename, possibly using dfd as a starting
// point." - fs/utimes.c
var path fspath.Path
shouldAllowEmptyPath := allowEmptyPath
if dirfd == linux.AT_FDCWD || pathAddr != 0 {
var err error
path, err = copyInPath(t, pathAddr)
if err != nil {
return 0, nil, err
}
shouldAllowEmptyPath = disallowEmptyPath
}
var opts vfs.SetStatOptions
if err := populateSetStatOptionsForUtimes(t, timesAddr, &opts); err != nil {
return 0, nil, err
}
return 0, nil, setstatat(t, dirfd, path, shouldAllowEmptyPath, followFinalSymlink, &opts)
return 0, nil, utimes(t, dirfd, pathAddr, followFinalSymlink, &opts)
}
func populateSetStatOptionsForUtimes(t *kernel.Task, timesAddr hostarch.Addr, opts *vfs.SetStatOptions) error {
@@ -1485,8 +1462,7 @@ func Utimensat(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintp
timesAddr := args[2].Pointer()
flags := args[3].Int()
// Linux requires that the UTIME_OMIT check occur before checking path or
// flags.
// Linux requires that the UTIME_OMIT check occur before flags.
var opts vfs.SetStatOptions
if err := populateSetStatOptionsForUtimens(t, timesAddr, &opts); err != nil {
return 0, nil, err
@@ -1499,21 +1475,7 @@ func Utimensat(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintp
return 0, nil, linuxerr.EINVAL
}
// "If filename is NULL and dfd refers to an open file, then operate on the
// file. Otherwise look up filename, possibly using dfd as a starting
// point." - fs/utimes.c
var path fspath.Path
shouldAllowEmptyPath := allowEmptyPath
if dirfd == linux.AT_FDCWD || pathAddr != 0 {
var err error
path, err = copyInPath(t, pathAddr)
if err != nil {
return 0, nil, err
}
shouldAllowEmptyPath = disallowEmptyPath
}
return 0, nil, setstatat(t, dirfd, path, shouldAllowEmptyPath, shouldFollowFinalSymlink(flags&linux.AT_SYMLINK_NOFOLLOW == 0), &opts)
return 0, nil, utimes(t, dirfd, pathAddr, shouldFollowFinalSymlink(flags&linux.AT_SYMLINK_NOFOLLOW == 0), &opts)
}
func populateSetStatOptionsForUtimens(t *kernel.Task, timesAddr hostarch.Addr, opts *vfs.SetStatOptions) error {
@@ -1550,6 +1512,27 @@ func populateSetStatOptionsForUtimens(t *kernel.Task, timesAddr hostarch.Addr, o
return nil
}
// Analogous to fs/utimes.c:do_utimes().
func utimes(t *kernel.Task, dirfd int32, pathAddr hostarch.Addr, shouldFollowFinalSymlink shouldFollowFinalSymlink, opts *vfs.SetStatOptions) error {
// "If filename is NULL and dfd refers to an open file, then operate on the
// file. Otherwise look up filename, possibly using dfd as a starting
// point." - fs/utimes.c:do_utimes()
if dirfd != linux.AT_FDCWD && pathAddr == 0 {
file := t.GetFile(dirfd)
if file == nil {
return linuxerr.EBADF
}
defer file.DecRef(t)
return file.SetStat(t, *opts)
}
path, err := copyInPath(t, pathAddr)
if err != nil {
return err
}
return setstatat(t, dirfd, path, disallowEmptyPath, shouldFollowFinalSymlink, opts)
}
// Rename implements Linux syscall rename(2).
func Rename(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) {
oldpathAddr := args[0].Pointer()
test/syscalls/linux/chown.cc
+8
View File
@@ -112,6 +112,14 @@ TEST(ChownTest, FchownatWithOpath) {
SyscallSucceeds());
}
TEST(ChownTest, FchownatWithOpathEmtpyPath) {
const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const auto dirfd =
ASSERT_NO_ERRNO_AND_VALUE(Open(dir.path(), O_DIRECTORY | O_PATH));
ASSERT_THAT(fchownat(dirfd.get(), "", geteuid(), getegid(), AT_EMPTY_PATH),
SyscallSucceeds());
}
TEST(ChownTest, FchownatEmptyPath) {
const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const auto fd =
test/syscalls/linux/utimes.cc
+35 -5
View File
@@ -21,6 +21,7 @@
#include <unistd.h>
#include <utime.h>
#include <cerrno>
#include <string>
#include "absl/time/time.h"
@@ -112,20 +113,29 @@ TEST(UtimesTest, MissingPath) {
void TestFutimesat(int dirFd, std::string const& path) {
struct stat statbuf;
const char* path_or_null = nullptr;
if (!path.empty()) path_or_null = path.c_str();
struct timeval times[2] = {{10, 0}, {20, 0}};
EXPECT_THAT(futimesat(dirFd, path.c_str(), times), SyscallSucceeds());
EXPECT_THAT(fstatat(dirFd, path.c_str(), &statbuf, 0), SyscallSucceeds());
EXPECT_THAT(futimesat(dirFd, path_or_null, times), SyscallSucceeds());
if (path_or_null) {
EXPECT_THAT(fstatat(dirFd, path_or_null, &statbuf, 0), SyscallSucceeds());
} else {
EXPECT_THAT(fstat(dirFd, &statbuf), SyscallSucceeds());
}
EXPECT_EQ(10, statbuf.st_atime);
EXPECT_EQ(20, statbuf.st_mtime);
absl::Time before;
absl::Time after;
TimeBoxed(&before, &after, [&] {
EXPECT_THAT(futimesat(dirFd, path.c_str(), nullptr), SyscallSucceeds());
EXPECT_THAT(futimesat(dirFd, path_or_null, nullptr), SyscallSucceeds());
});
EXPECT_THAT(fstatat(dirFd, path.c_str(), &statbuf, 0), SyscallSucceeds());
if (path_or_null) {
EXPECT_THAT(fstatat(dirFd, path_or_null, &statbuf, 0), SyscallSucceeds());
} else {
EXPECT_THAT(fstat(dirFd, &statbuf), SyscallSucceeds());
}
absl::Time atime = absl::TimeFromTimespec(statbuf.st_atim);
EXPECT_GE(atime, before);
@@ -150,6 +160,26 @@ TEST(FutimesatTest, OnRelPath) {
TestFutimesat(dirFd.get(), basename);
}
TEST(FutimesatTest, OnNullPath) {
auto f = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(f.path(), O_RDONLY));
TestFutimesat(fd.get(), "");
}
TEST(FutimesatTest, OnNullPathWithOPath) {
auto f = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(f.path(), O_PATH));
struct timeval times[2] = {{10, 0}, {20, 0}};
EXPECT_THAT(futimesat(fd.get(), nullptr, times),
SyscallFailsWithErrno(EBADF));
}
TEST(FutimesatTest, OnNullPathWithCWD) {
struct timeval times[2] = {{10, 0}, {20, 0}};
EXPECT_THAT(futimesat(AT_FDCWD, nullptr, times),
SyscallFailsWithErrno(EFAULT));
}
TEST(FutimesatTest, InvalidNsec) {
auto f = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
struct timeval times[4][2] = {{