From d93d19fd4eefdfd868919a73c9498e7da7eb9258 Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Wed, 10 Apr 2019 16:35:22 -0700 Subject: Fix uses of RootFromContext. RootFromContext can return a dirent with reference taken, or nil. We must call DecRef if (and only if) a real dirent is returned. PiperOrigin-RevId: 242965515 Change-Id: Ie2b7b4cb19ee09b6ccf788b71f3fd7efcdf35a11 --- pkg/sentry/fs/copy_up.go | 10 +++++++--- pkg/sentry/fs/file.go | 6 +++++- pkg/sentry/fs/file_overlay.go | 10 ++++++++-- pkg/sentry/fs/fsutil/file.go | 4 +++- pkg/sentry/fs/gofer/file.go | 4 +++- pkg/sentry/fs/host/file.go | 4 +++- pkg/sentry/fs/proc/fds.go | 4 +++- pkg/sentry/fs/proc/proc.go | 4 +++- pkg/sentry/fs/proc/task.go | 4 +++- pkg/sentry/fs/ramfs/dir.go | 4 +++- pkg/sentry/fs/tty/dir.go | 4 +++- pkg/sentry/kernel/kernel.go | 4 ++++ 12 files changed, 48 insertions(+), 14 deletions(-) diff --git a/pkg/sentry/fs/copy_up.go b/pkg/sentry/fs/copy_up.go index 6d4ebaaa4..ba69e718d 100644 --- a/pkg/sentry/fs/copy_up.go +++ b/pkg/sentry/fs/copy_up.go @@ -188,11 +188,15 @@ func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error { var childUpperInode *Inode parentUpper := parent.Inode.overlay.upper + root := RootFromContext(ctx) + if root != nil { + defer root.DecRef() + } // Create the file in the upper filesystem and get an Inode for it. switch next.Inode.StableAttr.Type { case RegularFile: - childFile, err := parentUpper.Create(ctx, RootFromContext(ctx), next.name, FileFlags{Read: true, Write: true}, attrs.Perms) + childFile, err := parentUpper.Create(ctx, root, next.name, FileFlags{Read: true, Write: true}, attrs.Perms) if err != nil { log.Warningf("copy up failed to create file: %v", err) return syserror.EIO @@ -201,7 +205,7 @@ func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error { childUpperInode = childFile.Dirent.Inode case Directory: - if err := parentUpper.CreateDirectory(ctx, RootFromContext(ctx), next.name, attrs.Perms); err != nil { + if err := parentUpper.CreateDirectory(ctx, root, next.name, attrs.Perms); err != nil { log.Warningf("copy up failed to create directory: %v", err) return syserror.EIO } @@ -221,7 +225,7 @@ func copyUpLocked(ctx context.Context, parent *Dirent, next *Dirent) error { log.Warningf("copy up failed to read symlink value: %v", err) return syserror.EIO } - if err := parentUpper.CreateLink(ctx, RootFromContext(ctx), link, next.name); err != nil { + if err := parentUpper.CreateLink(ctx, root, link, next.name); err != nil { log.Warningf("copy up failed to create symlink: %v", err) return syserror.EIO } diff --git a/pkg/sentry/fs/file.go b/pkg/sentry/fs/file.go index d66813103..01c18647c 100644 --- a/pkg/sentry/fs/file.go +++ b/pkg/sentry/fs/file.go @@ -410,7 +410,11 @@ func (f *File) ConfigureMMap(ctx context.Context, opts *memmap.MMapOpts) error { // MappedName implements memmap.MappingIdentity.MappedName. func (f *File) MappedName(ctx context.Context) string { - name, _ := f.Dirent.FullName(RootFromContext(ctx)) + root := RootFromContext(ctx) + if root != nil { + defer root.DecRef() + } + name, _ := f.Dirent.FullName(root) return name } diff --git a/pkg/sentry/fs/file_overlay.go b/pkg/sentry/fs/file_overlay.go index cd231bdef..4efe85832 100644 --- a/pkg/sentry/fs/file_overlay.go +++ b/pkg/sentry/fs/file_overlay.go @@ -169,7 +169,9 @@ func (f *overlayFileOperations) Seek(ctx context.Context, file *File, whence See // Readdir implements FileOperations.Readdir. func (f *overlayFileOperations) Readdir(ctx context.Context, file *File, serializer DentrySerializer) (int64, error) { root := RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &DirCtx{ Serializer: serializer, DirCursor: &f.dirCursor, @@ -440,7 +442,11 @@ func (omi *overlayMappingIdentity) InodeID() uint64 { // MappedName implements MappingIdentity.MappedName. func (omi *overlayMappingIdentity) MappedName(ctx context.Context) string { - name, _ := omi.overlayFile.Dirent.FullName(RootFromContext(ctx)) + root := RootFromContext(ctx) + if root != nil { + defer root.DecRef() + } + name, _ := omi.overlayFile.Dirent.FullName(root) return name } diff --git a/pkg/sentry/fs/fsutil/file.go b/pkg/sentry/fs/fsutil/file.go index 32f8133fb..ce329b37a 100644 --- a/pkg/sentry/fs/fsutil/file.go +++ b/pkg/sentry/fs/fsutil/file.go @@ -277,7 +277,9 @@ func (sdfo *StaticDirFileOperations) IterateDir(ctx context.Context, dirCtx *fs. // Readdir implements fs.FileOperations.Readdir. func (sdfo *StaticDirFileOperations) Readdir(ctx context.Context, file *fs.File, serializer fs.DentrySerializer) (int64, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &fs.DirCtx{ Serializer: serializer, DirCursor: &sdfo.dirCursor, diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go index 631cc80ae..e49ae2201 100644 --- a/pkg/sentry/fs/gofer/file.go +++ b/pkg/sentry/fs/gofer/file.go @@ -120,7 +120,9 @@ func (f *fileOperations) Release() { // Readdir implements fs.FileOperations.Readdir. func (f *fileOperations) Readdir(ctx context.Context, file *fs.File, serializer fs.DentrySerializer) (int64, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &fs.DirCtx{ Serializer: serializer, diff --git a/pkg/sentry/fs/host/file.go b/pkg/sentry/fs/host/file.go index 4e84d1d6c..d67a0795f 100644 --- a/pkg/sentry/fs/host/file.go +++ b/pkg/sentry/fs/host/file.go @@ -166,7 +166,9 @@ func (f *fileOperations) Readiness(mask waiter.EventMask) waiter.EventMask { // Readdir implements fs.FileOperations.Readdir. func (f *fileOperations) Readdir(ctx context.Context, file *fs.File, serializer fs.DentrySerializer) (int64, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &fs.DirCtx{ Serializer: serializer, DirCursor: &f.dirCursor, diff --git a/pkg/sentry/fs/proc/fds.go b/pkg/sentry/fs/proc/fds.go index b8a0a5eff..3c471bad9 100644 --- a/pkg/sentry/fs/proc/fds.go +++ b/pkg/sentry/fs/proc/fds.go @@ -119,7 +119,9 @@ func (f *fd) GetFile(context.Context, *fs.Dirent, fs.FileFlags) (*fs.File, error // Readlink returns the current target. func (f *fd) Readlink(ctx context.Context, _ *fs.Inode) (string, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } n, _ := f.Dirent.FullName(root) return n, nil } diff --git a/pkg/sentry/fs/proc/proc.go b/pkg/sentry/fs/proc/proc.go index 88018e707..c9e659533 100644 --- a/pkg/sentry/fs/proc/proc.go +++ b/pkg/sentry/fs/proc/proc.go @@ -210,7 +210,9 @@ func (rpf *rootProcFile) Readdir(ctx context.Context, file *fs.File, ser fs.Dent // Add dot and dotdot. root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dot, dotdot := file.Dirent.GetDotAttrs(root) names = append(names, ".", "..") m["."] = dot diff --git a/pkg/sentry/fs/proc/task.go b/pkg/sentry/fs/proc/task.go index 5a90c5578..4b1f84942 100644 --- a/pkg/sentry/fs/proc/task.go +++ b/pkg/sentry/fs/proc/task.go @@ -162,7 +162,9 @@ func (f *subtasksFile) Readdir(ctx context.Context, file *fs.File, ser fs.Dentry if offset == 0 { // Serialize "." and "..". root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dot, dotdot := file.Dirent.GetDotAttrs(root) if err := dirCtx.DirEmit(".", dot); err != nil { return offset, err diff --git a/pkg/sentry/fs/ramfs/dir.go b/pkg/sentry/fs/ramfs/dir.go index 05d716afb..a3b33c0f8 100644 --- a/pkg/sentry/fs/ramfs/dir.go +++ b/pkg/sentry/fs/ramfs/dir.go @@ -417,7 +417,9 @@ func (dfo *dirFileOperations) IterateDir(ctx context.Context, dirCtx *fs.DirCtx, // Readdir implements FileOperations.Readdir. func (dfo *dirFileOperations) Readdir(ctx context.Context, file *fs.File, serializer fs.DentrySerializer) (int64, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &fs.DirCtx{ Serializer: serializer, DirCursor: &dfo.dirCursor, diff --git a/pkg/sentry/fs/tty/dir.go b/pkg/sentry/fs/tty/dir.go index 485cdb456..11bf736d6 100644 --- a/pkg/sentry/fs/tty/dir.go +++ b/pkg/sentry/fs/tty/dir.go @@ -315,7 +315,9 @@ func (df *dirFileOperations) IterateDir(ctx context.Context, dirCtx *fs.DirCtx, // Readdir implements FileOperations.Readdir. func (df *dirFileOperations) Readdir(ctx context.Context, file *fs.File, serializer fs.DentrySerializer) (int64, error) { root := fs.RootFromContext(ctx) - defer root.DecRef() + if root != nil { + defer root.DecRef() + } dirCtx := &fs.DirCtx{ Serializer: serializer, DirCursor: &df.dirCursor, diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index a9994f23b..b8953657c 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -676,6 +676,10 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID, root := args.Root if root == nil { root = fs.RootFromContext(ctx) + // Is the root STILL nil? + if root == nil { + return nil, 0, fmt.Errorf("CreateProcessArgs.Root was not provided, and failed to get root from context") + } } defer root.DecRef() args.Root = nil -- cgit v1.2.3