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

Fix fsgofer Open() when control file is using O_PATH

Browse Source 
Open tries to reuse the control file to save syscalls and
file descriptors when opening a file. However, when the
control file was opened using O_PATH (e.g. no file permission
to open readonly), Open() would not check for it.

PiperOrigin-RevId: 322821729
This commit is contained in:
Fabricio Voznika
2020-07-23 11:02:35 -07:00
committed by gVisor bot
parent b396d3882c
commit 384369e01e
3 changed files with 128 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runsc/fsgofer/BUILD
+1
View File
@@ -31,5 +31,6 @@ go_test(
deps = [
"//pkg/log",
"//pkg/p9",
"//pkg/test/testutil",
],
)
runsc/fsgofer/fsgofer.go
+51 -37
View File
@@ -132,7 +132,7 @@ func (a *attachPoint) Attach() (p9.File, error) {
return nil, fmt.Errorf("attach point already attached, prefix: %s", a.prefix)
}
f, err := openAnyFile(a.prefix, func(mode int) (*fd.FD, error) {
f, readable, err := openAnyFile(a.prefix, func(mode int) (*fd.FD, error) {
return fd.Open(a.prefix, openFlags|mode, 0)
})
if err != nil {
@@ -144,7 +144,7 @@ func (a *attachPoint) Attach() (p9.File, error) {
return nil, fmt.Errorf("unable to stat %q: %v", a.prefix, err)
}
lf, err := newLocalFile(a, f, a.prefix, stat)
lf, err := newLocalFile(a, f, a.prefix, readable, stat)
if err != nil {
return nil, fmt.Errorf("unable to create localFile %q: %v", a.prefix, err)
}
@@ -212,6 +212,10 @@ type localFile struct {
// opened with.
file *fd.FD
// controlReadable tells whether 'file' was opened with read permissions
// during a walk.
controlReadable bool
// mode is the mode in which the file was opened. Set to invalidMode
// if localFile isn't opened.
mode p9.OpenFlags
@@ -251,49 +255,57 @@ func reopenProcFd(f *fd.FD, mode int) (*fd.FD, error) {
return fd.New(d), nil
}
func openAnyFileFromParent(parent *localFile, name string) (*fd.FD, string, error) {
func openAnyFileFromParent(parent *localFile, name string) (*fd.FD, string, bool, error) {
path := path.Join(parent.hostPath, name)
f, err := openAnyFile(path, func(mode int) (*fd.FD, error) {
f, readable, err := openAnyFile(path, func(mode int) (*fd.FD, error) {
return fd.OpenAt(parent.file, name, openFlags|mode, 0)
})
return f, path, err
return f, path, readable, err
}
// openAnyFile attempts to open the file in O_RDONLY and if it fails fallsback
// to O_PATH. 'path' is used for logging messages only. 'fn' is what does the
// actual file open and is customizable by the caller.
func openAnyFile(path string, fn func(mode int) (*fd.FD, error)) (*fd.FD, error) {
func openAnyFile(path string, fn func(mode int) (*fd.FD, error)) (*fd.FD, bool, error) {
// Attempt to open file in the following mode in order:
// 1. RDONLY | NONBLOCK: for all files, directories, ro mounts, FIFOs.
// Use non-blocking to prevent getting stuck inside open(2) for
// FIFOs. This option has no effect on regular files.
// 2. PATH: for symlinks, sockets.
modes := []int{syscall.O_RDONLY | syscall.O_NONBLOCK, unix.O_PATH}
options := []struct {
mode int
readable bool
}{
{
mode: syscall.O_RDONLY | syscall.O_NONBLOCK,
readable: true,
},
{
mode: unix.O_PATH,
readable: false,
},
}
var err error
var file *fd.FD
for i, mode := range modes {
file, err = fn(mode)
for i, option := range options {
var file *fd.FD
file, err = fn(option.mode)
if err == nil {
// openat succeeded, we're done.
break
// Succeeded opening the file, we're done.
return file, option.readable, nil
}
switch e := extractErrno(err); e {
case syscall.ENOENT:
// File doesn't exist, no point in retrying.
return nil, e
return nil, false, e
}
// openat failed. Try again with next mode, preserving 'err' in case this
// was the last attempt.
log.Debugf("Attempt %d to open file failed, mode: %#x, path: %q, err: %v", i, openFlags|mode, path, err)
// File failed to open. Try again with next mode, preserving 'err' in case
// this was the last attempt.
log.Debugf("Attempt %d to open file failed, mode: %#x, path: %q, err: %v", i, openFlags|option.mode, path, err)
}
if err != nil {
// All attempts to open file have failed, return the last error.
log.Debugf("Failed to open file, path: %q, err: %v", path, err)
return nil, extractErrno(err)
}
return file, nil
// All attempts to open file have failed, return the last error.
log.Debugf("Failed to open file, path: %q, err: %v", path, err)
return nil, false, extractErrno(err)
}
func getSupportedFileType(stat syscall.Stat_t, permitSocket bool) (fileType, error) {
@@ -316,18 +328,19 @@ func getSupportedFileType(stat syscall.Stat_t, permitSocket bool) (fileType, err
return ft, nil
}
func newLocalFile(a *attachPoint, file *fd.FD, path string, stat syscall.Stat_t) (*localFile, error) {
func newLocalFile(a *attachPoint, file *fd.FD, path string, readable bool, stat syscall.Stat_t) (*localFile, error) {
ft, err := getSupportedFileType(stat, a.conf.HostUDS)
if err != nil {
return nil, err
}
return &localFile{
attachPoint: a,
hostPath: path,
file: file,
mode: invalidMode,
ft: ft,
attachPoint: a,
hostPath: path,
file: file,
mode: invalidMode,
ft: ft,
controlReadable: readable,
}, nil
}
@@ -380,7 +393,7 @@ func (l *localFile) Open(flags p9.OpenFlags) (*fd.FD, p9.QID, uint32, error) {
// Check if control file can be used or if a new open must be created.
var newFile *fd.FD
if flags == p9.ReadOnly {
if flags == p9.ReadOnly && l.controlReadable {
log.Debugf("Open reusing control file, flags: %v, %q", flags, l.hostPath)
newFile = l.file
} else {
@@ -518,7 +531,7 @@ func (l *localFile) Mkdir(name string, perm p9.FileMode, uid p9.UID, gid p9.GID)
func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
// Duplicate current file if 'names' is empty.
if len(names) == 0 {
newFile, err := openAnyFile(l.hostPath, func(mode int) (*fd.FD, error) {
newFile, readable, err := openAnyFile(l.hostPath, func(mode int) (*fd.FD, error) {
return reopenProcFd(l.file, openFlags|mode)
})
if err != nil {
@@ -532,10 +545,11 @@ func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
}
c := &localFile{
attachPoint: l.attachPoint,
hostPath: l.hostPath,
file: newFile,
mode: invalidMode,
attachPoint: l.attachPoint,
hostPath: l.hostPath,
file: newFile,
mode: invalidMode,
controlReadable: readable,
}
return []p9.QID{l.attachPoint.makeQID(stat)}, c, nil
}
@@ -543,7 +557,7 @@ func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
var qids []p9.QID
last := l
for _, name := range names {
f, path, err := openAnyFileFromParent(last, name)
f, path, readable, err := openAnyFileFromParent(last, name)
if last != l {
last.Close()
}
@@ -555,7 +569,7 @@ func (l *localFile) Walk(names []string) ([]p9.QID, p9.File, error) {
f.Close()
return nil, nil, extractErrno(err)
}
c, err := newLocalFile(last.attachPoint, f, path, stat)
c, err := newLocalFile(last.attachPoint, f, path, readable, stat)
if err != nil {
f.Close()
return nil, nil, extractErrno(err)
runsc/fsgofer/fsgofer_test.go
+76 -38
View File
@@ -26,6 +26,19 @@ import (
"gvisor.dev/gvisor/pkg/log"
"gvisor.dev/gvisor/pkg/p9"
"gvisor.dev/gvisor/pkg/test/testutil"
)
var allOpenFlags = []p9.OpenFlags{p9.ReadOnly, p9.WriteOnly, p9.ReadWrite}
var (
allTypes = []fileType{regular, directory, symlink}
// allConfs is set in init().
allConfs []Config
rwConfs = []Config{{ROMount: false}}
roConfs = []Config{{ROMount: true}}
)
func init() {
@@ -39,6 +52,13 @@ func init() {
}
}
func configTestName(config *Config) string {
if config.ROMount {
return "ROMount"
}
return "RWMount"
}
func assertPanic(t *testing.T, f func()) {
defer func() {
if r := recover(); r == nil {
@@ -88,18 +108,6 @@ func testReadWrite(f p9.File, flags p9.OpenFlags, content []byte) error {
return nil
}
var allOpenFlags = []p9.OpenFlags{p9.ReadOnly, p9.WriteOnly, p9.ReadWrite}
var (
allTypes = []fileType{regular, directory, symlink}
// allConfs is set in init() above.
allConfs []Config
rwConfs = []Config{{ROMount: false}}
roConfs = []Config{{ROMount: true}}
)
type state struct {
root *localFile
file *localFile
@@ -117,42 +125,46 @@ func runAll(t *testing.T, test func(*testing.T, state)) {
func runCustom(t *testing.T, types []fileType, confs []Config, test func(*testing.T, state)) {
for _, c := range confs {
t.Logf("Config: %+v", c)
for _, ft := range types {
t.Logf("File type: %v", ft)
name := fmt.Sprintf("%s/%v", configTestName(&c), ft)
t.Run(name, func(t *testing.T) {
path, name, err := setup(ft)
if err != nil {
t.Fatalf("%v", err)
}
defer os.RemoveAll(path)
path, name, err := setup(ft)
if err != nil {
t.Fatalf("%v", err)
}
defer os.RemoveAll(path)
a, err := NewAttachPoint(path, c)
if err != nil {
t.Fatalf("NewAttachPoint failed: %v", err)
}
root, err := a.Attach()
if err != nil {
t.Fatalf("Attach failed, err: %v", err)
}
a, err := NewAttachPoint(path, c)
if err != nil {
t.Fatalf("NewAttachPoint failed: %v", err)
}
root, err := a.Attach()
if err != nil {
t.Fatalf("Attach failed, err: %v", err)
}
_, file, err := root.Walk([]string{name})
if err != nil {
root.Close()
t.Fatalf("root.Walk({%q}) failed, err: %v", "symlink", err)
}
_, file, err := root.Walk([]string{name})
if err != nil {
st := state{
root: root.(*localFile),
file: file.(*localFile),
conf: c,
ft: ft,
}
test(t, st)
file.Close()
root.Close()
t.Fatalf("root.Walk({%q}) failed, err: %v", "symlink", err)
}
st := state{root: root.(*localFile), file: file.(*localFile), conf: c, ft: ft}
test(t, st)
file.Close()
root.Close()
})
}
}
}
func setup(ft fileType) (string, string, error) {
path, err := ioutil.TempDir("", "root-")
path, err := ioutil.TempDir(testutil.TmpDir(), "root-")
if err != nil {
return "", "", fmt.Errorf("ioutil.TempDir() failed, err: %v", err)
}
@@ -308,6 +320,32 @@ func TestUnopened(t *testing.T) {
})
}
// TestOpenOPath is a regression test to ensure that a file that cannot be open
// for read is allowed to be open. This was happening because the control file
// was open with O_PATH, but Open() was not checking for it and allowing the
// control file to be reused.
func TestOpenOPath(t *testing.T) {
runCustom(t, []fileType{regular}, rwConfs, func(t *testing.T, s state) {
// Fist remove all permissions on the file.
if err := s.file.SetAttr(p9.SetAttrMask{Permissions: true}, p9.SetAttr{Permissions: p9.FileMode(0)}); err != nil {
t.Fatalf("SetAttr(): %v", err)
}
// Then walk to the file again to open a new control file.
filename := filepath.Base(s.file.hostPath)
_, newFile, err := s.root.Walk([]string{filename})
if err != nil {
t.Fatalf("root.Walk(%q): %v", filename, err)
}
if newFile.(*localFile).controlReadable {
t.Fatalf("control file didn't open with O_PATH: %+v", newFile)
}
if _, _, _, err := newFile.Open(p9.ReadOnly); err != syscall.EACCES {
t.Fatalf("Open() should have failed, got: %v, wanted: EACCES", err)
}
})
}
func SetGetAttr(l *localFile, valid p9.SetAttrMask, attr p9.SetAttr) (p9.Attr, error) {
if err := l.SetAttr(valid, attr); err != nil {
return p9.Attr{}, err