proc: fix security checks on /proc/pid files

This change updates the security checks to align with Linux kernel behavior:
* Add ptrace_may_access-style checks (kernel.ContextCanTrace) when accessing
  /proc/[pid]/maps and /proc/[pid]/smaps.
* Change the permissions of the /proc/[pid]/fd directory to 0500 (read/execute
  for owner only).
* Add a similar kernel.ContextCanTrace check for /proc/[pid]/{environ,auxv}.

PiperOrigin-RevId: 842001353

Author: avagin

pkg/sentry/fsimpl/cgroupfs/base.go

@@ -353,7 +353,11 @@ func (c *cgroupInode) ReadControl(ctx context.Context, name string) (string, err
 	}
 
 	var buf bytes.Buffer
-	err = cbf.Source().Data().Generate(ctx, &buf)
+	data, err := cbf.Source().Data(ctx)
+	if err != nil {
+		return "", err
+	}
+	err = data.Generate(ctx, &buf)
 	return buf.String(), err
 }
 

pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go

@@ -25,6 +25,11 @@ import (
 	"gvisor.dev/gvisor/pkg/usermem"
 )
 
+// DataSourceProvider represents a dynamic data source provider for DynamicBytesFile.
+type DataSourceProvider interface {
+	DataSource(ctx context.Context) (vfs.DynamicBytesSource, error)
+}
+
 // DynamicBytesFile implements kernfs.Inode and represents a read-only file
 // whose contents are backed by a vfs.DynamicBytesSource. If data additionally
 // implements vfs.WritableDynamicBytesSource, the file also supports dispatching
@@ -49,6 +54,10 @@ type DynamicBytesFile struct {
 	// writes. This field cannot be changed to a different bytes source after
 	// Init.
 	data vfs.DynamicBytesSource
+
+	// dataSourceProvider is used to get the data source when the file is opened.
+	// If it is nil, the data source is fixed to the value in the data field.
+	dataSourceProvider DataSourceProvider
 }
 
 var _ Inode = (*DynamicBytesFile)(nil)
@@ -64,8 +73,12 @@ func (f *DynamicBytesFile) Init(ctx context.Context, creds *auth.Credentials, de
 
 // Open implements Inode.Open.
 func (f *DynamicBytesFile) Open(ctx context.Context, rp *vfs.ResolvingPath, d *Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) {
+	data, err := f.Data(ctx)
+	if err != nil {
+		return nil, err
+	}
 	fd := &DynamicBytesFD{}
-	if err := fd.Init(rp.Mount(), d, f.data, &f.locks, opts.Flags); err != nil {
+	if err := fd.Init(rp.Mount(), d, data, &f.locks, opts.Flags); err != nil {
 		return nil, err
 	}
 	return &fd.vfsfd, nil
@@ -83,9 +96,17 @@ func (f *DynamicBytesFile) Locks() *vfs.FileLocks {
 	return &f.locks
 }
 
+// SetDataSourceProvider sets a DataSourceProvider for the file.
+func (f *DynamicBytesFile) SetDataSourceProvider(p DataSourceProvider) {
+	f.dataSourceProvider = p
+}
+
 // Data returns the underlying data source.
-func (f *DynamicBytesFile) Data() vfs.DynamicBytesSource {
-	return f.data
+func (f *DynamicBytesFile) Data(ctx context.Context) (vfs.DynamicBytesSource, error) {
+	if f.dataSourceProvider != nil {
+		return f.dataSourceProvider.DataSource(ctx)
+	}
+	return f.data, nil
 }
 
 // DynamicBytesFD implements vfs.FileDescriptionImpl for an FD backed by a

pkg/sentry/fsimpl/proc/task.go

@@ -59,18 +59,18 @@ func (fs *filesystem) newTaskInode(ctx context.Context, task *kernel.Task, pidns
 	}
 
 	contents := map[string]kernfs.Inode{
-		"auxv":      fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &auxvData{task: task}),
-		"cmdline":   fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &metadataData{task: task, metaType: Cmdline}),
+		"auxv":      fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0400, &mmFile{task: task, ftype: auxvMMFile}),
+		"cmdline":   fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &cmdlineData{task: task}),
 		"comm":      fs.newComm(ctx, task, fs.NextIno(), 0644),
 		"cwd":       fs.newCwdSymlink(ctx, task, fs.NextIno()),
-		"environ":   fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &metadataData{task: task, metaType: Environ}),
+		"environ":   fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0400, &mmFile{task: task, ftype: environMMFile}),
 		"exe":       fs.newExeSymlink(ctx, task, fs.NextIno()),
 		"fd":        fs.newFDDirInode(ctx, task),
 		"fdinfo":    fs.newFDInfoDirInode(ctx, task),
 		"gid_map":   fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0644, &idMapData{task: task, gids: true}),
 		"io":        fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0400, newIO(task, isThreadGroup)),
 		"limits":    fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &limitsData{task: task}),
-		"maps":      fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &mapsData{task: task}),
+		"maps":      fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &mmFile{task: task, ftype: mapsMMFile}),
 		"mem":       fs.newMemInode(ctx, task, fs.NextIno(), 0600),
 		"mountinfo": fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &mountInfoData{fs: fs, task: task}),
 		"mounts":    fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &mountsData{fs: fs, task: task}),
@@ -86,7 +86,7 @@ func (fs *filesystem) newTaskInode(ctx context.Context, task *kernel.Task, pidns
 		"oom_score":     fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, newStaticFile("0\n")),
 		"oom_score_adj": fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0644, &oomScoreAdj{task: task}),
 		"root":          fs.newRootSymlink(ctx, task, fs.NextIno()),
-		"smaps":         fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &smapsData{task: task}),
+		"smaps":         fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &mmFile{task: task, ftype: smapsMMFile}),
 		"stat":          fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &taskStatData{task: task, pidns: pidns, tgstats: isThreadGroup}),
 		"statm":         fs.newTaskOwnedInode(ctx, task, fs.NextIno(), 0444, &statmData{task: task}),
 		"status":        fs.newStatusInode(ctx, task, pidns, fs.NextIno(), 0444),
@@ -248,7 +248,7 @@ func (i *taskOwnedInode) Stat(ctx context.Context, fs *vfs.Filesystem, opts vfs.
 }
 
 // CheckPermissions implements kernfs.Inode.CheckPermissions.
-func (i *taskOwnedInode) CheckPermissions(_ context.Context, creds *auth.Credentials, ats vfs.AccessTypes) error {
+func (i *taskOwnedInode) CheckPermissions(ctx context.Context, creds *auth.Credentials, ats vfs.AccessTypes) error {
 	mode := i.Mode()
 	uid, gid := i.getOwner(mode)
 	return vfs.GenericCheckPermissions(creds, ats, mode, uid, gid)

pkg/sentry/fsimpl/proc/task_fds.go

@@ -131,7 +131,7 @@ func (fs *filesystem) newFDDirInode(ctx context.Context, task *kernel.Task) kern
 			produceSymlink: true,
 		},
 	}
-	inode.InodeAttrs.Init(ctx, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0555)
+	inode.InodeAttrs.Init(ctx, task.Credentials(), linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|0500)
 	inode.InitRefs()
 	inode.OrderedChildren.Init(kernfs.OrderedChildrenOptions{})
 	return inode
@@ -223,6 +223,9 @@ func (fs *filesystem) newFDSymlink(ctx context.Context, task *kernel.Task, fd in
 }
 
 func (s *fdSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error) {
+	if !kernel.ContextCanTrace(ctx, s.task, false) {
+		return "", linuxerr.EACCES
+	}
 	file, _ := getTaskFD(s.task, s.fd)
 	if file == nil {
 		return "", linuxerr.ENOENT
@@ -236,6 +239,9 @@ func (s *fdSymlink) Readlink(ctx context.Context, _ *vfs.Mount) (string, error)
 }
 
 func (s *fdSymlink) Getlink(ctx context.Context, mnt *vfs.Mount) (vfs.VirtualDentry, string, error) {
+	if !kernel.ContextCanTrace(ctx, s.task, false) {
+		return vfs.VirtualDentry{}, "", linuxerr.EACCES
+	}
 	file, _ := getTaskFD(s.task, s.fd)
 	if file == nil {
 		return vfs.VirtualDentry{}, "", linuxerr.ENOENT

pkg/sentry/fsimpl/proc/task_files.go

@@ -102,20 +102,13 @@ func (w *bufferWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, error) {
 //
 // +stateify savable
 type auxvData struct {
-	kernfs.DynamicBytesFile
-
-	task *kernel.Task
+	mm *mm.MemoryManager
 }
 
-var _ dynamicInode = (*auxvData)(nil)
-
 // Generate implements vfs.DynamicBytesSource.Generate.
 func (d *auxvData) Generate(ctx context.Context, buf *bytes.Buffer) error {
-	if d.task.ExitState() == kernel.TaskExitDead {
-		return linuxerr.ESRCH
-	}
-	m, err := getMMIncRef(d.task)
-	if err != nil {
+	m := d.mm
+	if m == nil || !m.IncUsers() {
 		// Return empty file.
 		return nil
 	}
@@ -228,25 +221,19 @@ func GetMetadata(ctx context.Context, mm *mm.MemoryManager, buf *bytes.Buffer, t
 	return nil
 }
 
-// metadataData implements vfs.DynamicBytesSource for proc metadata fields like:
-//
-//   - /proc/[pid]/cmdline
-//   - /proc/[pid]/environ
+// cmdlineData implements vfs.DynamicBytesSource for /proc/[pid]/cmdline:
 //
 // +stateify savable
-type metadataData struct {
+type cmdlineData struct {
 	kernfs.DynamicBytesFile
 
 	task *kernel.Task
-
-	// arg is the type of exec argument this file contains.
-	metaType MetadataType
 }
 
-var _ dynamicInode = (*metadataData)(nil)
+var _ dynamicInode = (*cmdlineData)(nil)
 
 // Generate implements vfs.DynamicBytesSource.Generate.
-func (d *metadataData) Generate(ctx context.Context, buf *bytes.Buffer) error {
+func (d *cmdlineData) Generate(ctx context.Context, buf *bytes.Buffer) error {
 	if d.task.ExitState() == kernel.TaskExitDead {
 		return linuxerr.ESRCH
 	}
@@ -256,7 +243,25 @@ func (d *metadataData) Generate(ctx context.Context, buf *bytes.Buffer) error {
 		return nil
 	}
 	defer m.DecUsers(ctx)
-	return GetMetadata(ctx, m, buf, d.metaType)
+	return GetMetadata(ctx, m, buf, Cmdline)
+}
+
+// environData implements vfs.DynamicBytesSource for /proc/[pid]/environ:
+//
+// +stateify savable
+type environData struct {
+	mm *mm.MemoryManager
+}
+
+// Generate implements vfs.DynamicBytesSource.Generate.
+func (d *environData) Generate(ctx context.Context, buf *bytes.Buffer) error {
+	m := d.mm
+	if m == nil || !m.IncUsers() {
+		// Return empty file.
+		return nil
+	}
+	defer m.DecUsers(ctx)
+	return GetMetadata(ctx, m, buf, Environ)
 }
 
 // +stateify savable
@@ -444,11 +449,20 @@ func (f *memInode) init(ctx context.Context, creds *auth.Credentials, devMajor,
 
 // Open implements kernfs.Inode.Open.
 func (f *memInode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) {
-	// TODO(gvisor.dev/issue/260): Add check for PTRACE_MODE_ATTACH_FSCREDS
-	// Permission to read this file is governed by PTRACE_MODE_ATTACH_FSCREDS
-	// Since we dont implement setfsuid/setfsgid we can just use PTRACE_MODE_ATTACH
-	if !kernel.ContextCanTrace(ctx, f.task, true) {
-		return nil, linuxerr.EACCES
+	m := getMM(f.task)
+	for {
+		// TODO(gvisor.dev/issue/260): Add check for PTRACE_MODE_ATTACH_FSCREDS
+		// Permission to read this file is governed by PTRACE_MODE_ATTACH_FSCREDS
+		// Since we dont implement setfsuid/setfsgid we can just use PTRACE_MODE_ATTACH
+		if !kernel.ContextCanTrace(ctx, f.task, true) {
+			return nil, linuxerr.EACCES
+		}
+		// Check that the task still has the same mm (hasn't called execve).
+		m2 := getMM(f.task)
+		if m == m2 {
+			break
+		}
+		m = m2
 	}
 	if err := checkTaskState(f.task); err != nil {
 		return nil, err
@@ -624,7 +638,6 @@ func (d *limitsData) Generate(ctx context.Context, buf *bytes.Buffer) error {
 		} else {
 			fmt.Fprintf(buf, "%-20d ", l.Max)
 		}
-
 		if u := lt.Unit(); u != "" {
 			fmt.Fprintf(buf, "%-10s", u)
 		}
@@ -634,41 +647,99 @@ func (d *limitsData) Generate(ctx context.Context, buf *bytes.Buffer) error {
 	return nil
 }
 
-// mapsData implements vfs.DynamicBytesSource for /proc/[pid]/maps.
+// mmFile implements vfs.DynamicBytesSource for files that links with a process mm.
 //
 // +stateify savable
-type mapsData struct {
+type mmFile struct {
 	kernfs.DynamicBytesFile
 
-	task *kernel.Task
+	ftype mmFileType
+	task  *kernel.Task
 }
 
-var _ dynamicInode = (*mapsData)(nil)
+type mmFileType int
+
+const (
+	mapsMMFile mmFileType = iota
+	smapsMMFile
+	auxvMMFile
+	environMMFile
+	cmdlineMMFile
+)
+
+var _ dynamicInode = (*mmFile)(nil)
+
+func (f *mmFile) Init(ctx context.Context, creds *auth.Credentials, devMajor, devMinor uint32, ino uint64, data vfs.DynamicBytesSource, perm linux.FileMode) {
+	f.DynamicBytesFile.Init(ctx, creds, devMajor, devMinor, ino, nil, perm)
+	f.SetDataSourceProvider(f)
+}
+
+// DataSource implements kernfs.DataSourceProvider.DataSource()
+func (f *mmFile) DataSource(ctx context.Context) (vfs.DynamicBytesSource, error) {
+	m := getMM(f.task)
+	for {
+		if !kernel.ContextCanTrace(ctx, f.task, false) {
+			return nil, linuxerr.EACCES
+		}
+		// Check that the task still has the same mm (hasn't called execve).
+		m2 := getMM(f.task)
+		if m == m2 {
+			break
+		}
+		m = m2
+	}
+	switch f.ftype {
+	case mapsMMFile:
+		return &mapsData{mm: m}, nil
+	case smapsMMFile:
+		return &smapsData{mm: m}, nil
+	case environMMFile:
+		return &environData{mm: m}, nil
+	case auxvMMFile:
+		return &auxvData{mm: m}, nil
+	default:
+		panic(fmt.Sprintf("unknown file type: %d", f.ftype))
+	}
+}
+
+var _ kernfs.DataSourceProvider = (*mmFile)(nil)
+
+// Generate implements vfs.DynamicBytesSource.Generate.
+func (f *mmFile) Generate(ctx context.Context, buf *bytes.Buffer) error {
+	panic("unreachable")
+}
+
+// mapsData implements vfs.DynamicBytesSource for /proc/[pid]/maps.
+//
+// +stateify savable
+type mapsData struct {
+	mm *mm.MemoryManager
+}
 
 // Generate implements vfs.DynamicBytesSource.Generate.
 func (d *mapsData) Generate(ctx context.Context, buf *bytes.Buffer) error {
-	if mm := getMM(d.task); mm != nil {
-		mm.ReadMapsDataInto(ctx, mm.MapsCallbackFuncForBuffer(buf))
+	if d.mm == nil || !d.mm.IncUsers() {
+		return nil
 	}
+	defer d.mm.DecUsers(ctx)
+	d.mm.ReadMapsDataInto(ctx, d.mm.MapsCallbackFuncForBuffer(buf))
 	return nil
 }
 
 // smapsData implements vfs.DynamicBytesSource for /proc/[pid]/smaps.
 //
 // +stateify savable
 type smapsData struct {
-	kernfs.DynamicBytesFile
-
-	task *kernel.Task
+	mm *mm.MemoryManager
 }
 
-var _ dynamicInode = (*smapsData)(nil)
-
 // Generate implements vfs.DynamicBytesSource.Generate.
 func (d *smapsData) Generate(ctx context.Context, buf *bytes.Buffer) error {
-	if mm := getMM(d.task); mm != nil {
-		mm.ReadSmapsDataInto(ctx, buf)
+	if d.mm == nil || !d.mm.IncUsers() {
+		return nil
 	}
+	defer d.mm.DecUsers(ctx)
+	d.mm.ReadSmapsDataInto(ctx, buf)
 	return nil
 }
 

pkg/sentry/kernel/ptrace.go

@@ -123,7 +123,7 @@ func (t *Task) CanTrace(target *Task, attach bool) bool {
 		return false
 	}
 
-	if t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL {
+	if attach && t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL {
 		t.tg.pidns.owner.mu.RLock()
 		defer t.tg.pidns.owner.mu.RUnlock()
 		if !t.canTraceYAMALocked(target) {
@@ -144,7 +144,7 @@ func (t *Task) canTraceLocked(target *Task, attach bool) bool {
 		return false
 	}
 
-	if t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL {
+	if attach && t.k.YAMAPtraceScope.Load() == linux.YAMA_SCOPE_RELATIONAL {
 		if !t.canTraceYAMALocked(target) {
 			return false
 		}