summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--pkg/p9/file.go35
-rw-r--r--pkg/p9/handlers.go6
-rw-r--r--pkg/p9/p9test/BUILD2
-rw-r--r--pkg/p9/p9test/p9test.go1
-rw-r--r--pkg/p9/server.go9
-rw-r--r--runsc/fsgofer/fsgofer.go11
-rw-r--r--test/syscalls/BUILD4
-rw-r--r--test/syscalls/linux/BUILD15
-rw-r--r--test/syscalls/linux/deleted.cc116
-rw-r--r--test/util/fs_util.cc8
-rw-r--r--test/util/fs_util.h1
11 files changed, 200 insertions, 8 deletions
diff --git a/pkg/p9/file.go b/pkg/p9/file.go
index 8d6af2d6b..b4b556cb9 100644
--- a/pkg/p9/file.go
+++ b/pkg/p9/file.go
@@ -21,13 +21,37 @@ import (
"gvisor.dev/gvisor/pkg/fd"
)
+// AttacherOptions contains Attacher configuration.
+type AttacherOptions struct {
+ // SetAttrOnDeleted is set to true if it's safe to call File.SetAttr for
+ // deleted files.
+ SetAttrOnDeleted bool
+
+ // AllocateOnDeleted is set to true if it's safe to call File.Allocate for
+ // deleted files.
+ AllocateOnDeleted bool
+}
+
+// NoServerOptions partially implements Attacher with empty AttacherOptions.
+type NoServerOptions struct{}
+
+// ServerOptions implements Attacher.
+func (*NoServerOptions) ServerOptions() AttacherOptions {
+ return AttacherOptions{}
+}
+
// Attacher is provided by the server.
type Attacher interface {
// Attach returns a new File.
//
- // The client-side attach will be translate to a series of walks from
+ // The client-side attach will be translated to a series of walks from
// the file returned by this Attach call.
Attach() (File, error)
+
+ // ServerOptions returns configuration options for this attach point.
+ //
+ // This is never caller in the client-side.
+ ServerOptions() AttacherOptions
}
// File is a set of operations corresponding to a single node.
@@ -301,7 +325,7 @@ type File interface {
type DefaultWalkGetAttr struct{}
// WalkGetAttr implements File.WalkGetAttr.
-func (DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, error) {
+func (*DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, error) {
return nil, nil, AttrMask{}, Attr{}, unix.ENOSYS
}
@@ -309,7 +333,7 @@ func (DefaultWalkGetAttr) WalkGetAttr([]string) ([]QID, File, AttrMask, Attr, er
type DisallowClientCalls struct{}
// SetAttrClose implements File.SetAttrClose.
-func (DisallowClientCalls) SetAttrClose(SetAttrMask, SetAttr) error {
+func (*DisallowClientCalls) SetAttrClose(SetAttrMask, SetAttr) error {
panic("SetAttrClose should not be called on the server")
}
@@ -321,6 +345,11 @@ func (*DisallowServerCalls) Renamed(File, string) {
panic("Renamed should not be called on the client")
}
+// ServerOptions implements Attacher.
+func (*DisallowServerCalls) ServerOptions() AttacherOptions {
+ panic("ServerOptions should not be called on the client")
+}
+
// DefaultMultiGetAttr implements File.MultiGetAttr() on top of File.
func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
stats := make([]FullStat, 0, len(names))
diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go
index 2657081e3..c85af5e9e 100644
--- a/pkg/p9/handlers.go
+++ b/pkg/p9/handlers.go
@@ -178,7 +178,7 @@ func (t *Tsetattrclunk) handle(cs *connState) message {
// This might be technically incorrect, as it's possible that
// there were multiple links and you can still change the
// corresponding inode information.
- if ref.isDeleted() {
+ if !cs.server.options.SetAttrOnDeleted && ref.isDeleted() {
return unix.EINVAL
}
@@ -913,7 +913,7 @@ func (t *Tsetattr) handle(cs *connState) message {
// This might be technically incorrect, as it's possible that
// there were multiple links and you can still change the
// corresponding inode information.
- if ref.isDeleted() {
+ if !cs.server.options.SetAttrOnDeleted && ref.isDeleted() {
return unix.EINVAL
}
@@ -946,7 +946,7 @@ func (t *Tallocate) handle(cs *connState) message {
}
// We don't allow allocate on files that have been deleted.
- if ref.isDeleted() {
+ if !cs.server.options.AllocateOnDeleted && ref.isDeleted() {
return unix.EINVAL
}
diff --git a/pkg/p9/p9test/BUILD b/pkg/p9/p9test/BUILD
index 9c1ada0cb..f3eb8468b 100644
--- a/pkg/p9/p9test/BUILD
+++ b/pkg/p9/p9test/BUILD
@@ -12,7 +12,7 @@ MOCK_SRC_PACKAGE = "gvisor.dev/gvisor/pkg/p9"
# mockgen_reflect is a source file that contains mock generation code that
# imports the p9 package and generates a specification via reflection. The
# usual generation path must be split into two distinct parts because the full
-# source tree is not available to all build targets. Only declared depencies
+# source tree is not available to all build targets. Only declared dependencies
# are available (and even then, not the Go source files).
genrule(
name = "mockgen_reflect",
diff --git a/pkg/p9/p9test/p9test.go b/pkg/p9/p9test/p9test.go
index fd5ac3dbe..56939d100 100644
--- a/pkg/p9/p9test/p9test.go
+++ b/pkg/p9/p9test/p9test.go
@@ -307,6 +307,7 @@ func NewHarness(t *testing.T) (*Harness, *p9.Client) {
}
// Start the server, synchronized on exit.
+ h.Attacher.EXPECT().ServerOptions().Return(p9.AttacherOptions{}).Times(1)
server := p9.NewServer(h.Attacher)
h.wg.Add(1)
go func() {
diff --git a/pkg/p9/server.go b/pkg/p9/server.go
index 6428ad745..e7d129f9d 100644
--- a/pkg/p9/server.go
+++ b/pkg/p9/server.go
@@ -34,6 +34,8 @@ type Server struct {
// attacher provides the attach function.
attacher Attacher
+ options AttacherOptions
+
// pathTree is the full set of paths opened on this server.
//
// These may be across different connections, but rename operations
@@ -48,10 +50,15 @@ type Server struct {
renameMu sync.RWMutex
}
-// NewServer returns a new server.
+// NewServer returns a new server. attacher may be nil.
func NewServer(attacher Attacher) *Server {
+ opts := AttacherOptions{}
+ if attacher != nil {
+ opts = attacher.ServerOptions()
+ }
return &Server{
attacher: attacher,
+ options: opts,
pathTree: newPathNode(),
}
}
diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go
index 600b21189..3d610199c 100644
--- a/runsc/fsgofer/fsgofer.go
+++ b/runsc/fsgofer/fsgofer.go
@@ -140,6 +140,17 @@ func (a *attachPoint) Attach() (p9.File, error) {
return lf, nil
}
+// ServerOptions implements p9.Attacher. It's safe to call SetAttr and Allocate
+// on deleted files because fsgofer either uses an existing FD or opens a new
+// one using the magic symlink in `/proc/[pid]/fd` and cannot mistakely open
+// a file that was created in the same path as the delete file.
+func (a *attachPoint) ServerOptions() p9.AttacherOptions {
+ return p9.AttacherOptions{
+ SetAttrOnDeleted: true,
+ AllocateOnDeleted: true,
+ }
+}
+
// makeQID returns a unique QID for the given stat buffer.
func (a *attachPoint) makeQID(stat *unix.Stat_t) p9.QID {
a.deviceMu.Lock()
diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD
index f748d685a..7952fd969 100644
--- a/test/syscalls/BUILD
+++ b/test/syscalls/BUILD
@@ -1053,3 +1053,7 @@ syscall_test(
syscall_test(
test = "//test/syscalls/linux:verity_mount_test",
)
+
+syscall_test(
+ test = "//test/syscalls/linux:deleted_test",
+)
diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD
index 6217ff4dc..020c4673a 100644
--- a/test/syscalls/linux/BUILD
+++ b/test/syscalls/linux/BUILD
@@ -4432,3 +4432,18 @@ cc_binary(
"@com_google_absl//absl/container:flat_hash_set",
],
)
+
+cc_binary(
+ name = "deleted_test",
+ testonly = 1,
+ srcs = ["deleted.cc"],
+ linkstatic = 1,
+ deps = [
+ "//test/util:file_descriptor",
+ "//test/util:fs_util",
+ gtest,
+ "//test/util:temp_path",
+ "//test/util:test_main",
+ "//test/util:test_util",
+ ],
+)
diff --git a/test/syscalls/linux/deleted.cc b/test/syscalls/linux/deleted.cc
new file mode 100644
index 000000000..695ceafd3
--- /dev/null
+++ b/test/syscalls/linux/deleted.cc
@@ -0,0 +1,116 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <errno.h>
+#include <fcntl.h>
+#include <time.h>
+#include <unistd.h>
+
+#include <string>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "test/util/file_descriptor.h"
+#include "test/util/fs_util.h"
+#include "test/util/temp_path.h"
+#include "test/util/test_util.h"
+
+constexpr mode_t mode = 1;
+
+namespace gvisor {
+namespace testing {
+
+namespace {
+
+PosixErrorOr<FileDescriptor> createdDeleted() {
+ auto path = NewTempAbsPath();
+ PosixErrorOr<FileDescriptor> fd = Open(path, O_RDWR | O_CREAT, mode);
+ if (!fd.ok()) {
+ return fd.error();
+ }
+
+ auto err = Unlink(path);
+ if (!err.ok()) {
+ return err;
+ }
+ return fd;
+}
+
+TEST(DeletedTest, Utime) {
+ auto fd = ASSERT_NO_ERRNO_AND_VALUE(createdDeleted());
+
+ const struct timespec times[2] = {{10, 0}, {20, 0}};
+ EXPECT_THAT(futimens(fd.get(), times), SyscallSucceeds());
+
+ struct stat stat;
+ ASSERT_THAT(fstat(fd.get(), &stat), SyscallSucceeds());
+ EXPECT_EQ(10, stat.st_atime);
+ EXPECT_EQ(20, stat.st_mtime);
+}
+
+TEST(DeletedTest, Chmod) {
+ auto fd = ASSERT_NO_ERRNO_AND_VALUE(createdDeleted());
+
+ ASSERT_THAT(fchmod(fd.get(), mode + 1), SyscallSucceeds());
+
+ struct stat stat;
+ ASSERT_THAT(fstat(fd.get(), &stat), SyscallSucceeds());
+ EXPECT_EQ(mode + 1, stat.st_mode & ~S_IFMT);
+}
+
+TEST(DeletedTest, Truncate) {
+ auto fd = ASSERT_NO_ERRNO_AND_VALUE(createdDeleted());
+ const std::string data = "foobar";
+ ASSERT_THAT(write(fd.get(), data.c_str(), data.size()), SyscallSucceeds());
+
+ ASSERT_THAT(ftruncate(fd.get(), 0), SyscallSucceeds());
+
+ struct stat stat;
+ ASSERT_THAT(fstat(fd.get(), &stat), SyscallSucceeds());
+ ASSERT_EQ(stat.st_size, 0);
+}
+
+TEST(DeletedTest, Fallocate) {
+ auto fd = ASSERT_NO_ERRNO_AND_VALUE(createdDeleted());
+
+ ASSERT_THAT(fallocate(fd.get(), 0, 0, 123), SyscallSucceeds());
+
+ struct stat stat;
+ ASSERT_THAT(fstat(fd.get(), &stat), SyscallSucceeds());
+ EXPECT_EQ(123, stat.st_size);
+}
+
+// Tests that a file can be created with the same path as a deleted file that
+// still have an open FD to it.
+TEST(DeletedTest, Replace) {
+ auto path = NewTempAbsPath();
+ auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_RDWR | O_CREAT, mode));
+ ASSERT_NO_ERRNO(Unlink(path));
+
+ auto other =
+ ASSERT_NO_ERRNO_AND_VALUE(Open(path, O_RDWR | O_CREAT | O_EXCL, mode));
+
+ auto stat = ASSERT_NO_ERRNO_AND_VALUE(Fstat(fd.get()));
+ auto stat_other = ASSERT_NO_ERRNO_AND_VALUE(Fstat(other.get()));
+ ASSERT_NE(stat.st_ino, stat_other.st_ino);
+
+ // Check that the path points to the new file.
+ stat = ASSERT_NO_ERRNO_AND_VALUE(Stat(path));
+ ASSERT_EQ(stat.st_ino, stat_other.st_ino);
+}
+
+} // namespace
+
+} // namespace testing
+} // namespace gvisor
diff --git a/test/util/fs_util.cc b/test/util/fs_util.cc
index 253411858..1c24d9ffc 100644
--- a/test/util/fs_util.cc
+++ b/test/util/fs_util.cc
@@ -188,6 +188,14 @@ PosixError MknodAt(const FileDescriptor& dfd, absl::string_view path, int mode,
return NoError();
}
+PosixError Unlink(absl::string_view path) {
+ int res = unlink(std::string(path).c_str());
+ if (res < 0) {
+ return PosixError(errno, absl::StrCat("unlink ", path));
+ }
+ return NoError();
+}
+
PosixError UnlinkAt(const FileDescriptor& dfd, absl::string_view path,
int flags) {
int res = unlinkat(dfd.get(), std::string(path).c_str(), flags);
diff --git a/test/util/fs_util.h b/test/util/fs_util.h
index bb2d1d3c8..3ae0a725a 100644
--- a/test/util/fs_util.h
+++ b/test/util/fs_util.h
@@ -71,6 +71,7 @@ PosixError MknodAt(const FileDescriptor& dfd, absl::string_view path, int mode,
dev_t dev);
// Unlink the file.
+PosixError Unlink(absl::string_view path);
PosixError UnlinkAt(const FileDescriptor& dfd, absl::string_view path,
int flags);
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-01 06:38:01 +0000