On Wed, Jun 30, 2021 at 06:54:44PM -0700, Andrew Morton wrote: > From: Kalesh Singh <kaleshsingh@xxxxxxxxxx> > Subject: procfs: allow reading fdinfo with PTRACE_MODE_READ > > Android captures per-process system memory state when certain low memory > events (e.g a foreground app kill) occur, to identify potential memory > hoggers. In order to measure how much memory a process actually consumes, > it is necessary to include the DMA buffer sizes for that process in the > memory accounting. Since the handle to DMA buffers are raw FDs, it is > important to be able to identify which processes have FD references to a > DMA buffer. > > Currently, DMA buffer FDs can be accounted using /proc/<pid>/fd/* and > /proc/<pid>/fdinfo -- both are only readable by the process owner, as > follows: > > 1. Do a readlink on each FD. > 2. If the target path begins with "/dmabuf", then the FD is a dmabuf FD. > 3. stat the file to get the dmabuf inode number. > 4. Read/ proc/<pid>/fdinfo/<fd>, to get the DMA buffer size. > > Accessing other processes' fdinfo requires root privileges. This limits > the use of the interface to debugging environments and is not suitable for > production builds. Granting root privileges even to a system process > increases the attack surface and is highly undesirable. > > Since fdinfo doesn't permit reading process memory and manipulating > process state, allow accessing fdinfo under PTRACE_MODE_READ_FSCRED. > > Link: https://lkml.kernel.org/r/20210308170651.919148-1-kaleshsingh@xxxxxxxxxx > Signed-off-by: Kalesh Singh <kaleshsingh@xxxxxxxxxx> > Suggested-by: Jann Horn <jannh@xxxxxxxxxx> > Acked-by: Christian König <christian.koenig@xxxxxxx> > Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx> > Cc: Alexey Dobriyan <adobriyan@xxxxxxxxx> > Cc: Alexey Gladkov <gladkov.alexey@xxxxxxxxx> > Cc: Andrei Vagin <avagin@xxxxxxxxx> > Cc: Bernd Edlinger <bernd.edlinger@xxxxxxxxxx> > Cc: Christian Brauner <christian.brauner@xxxxxxxxxx> > Cc: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> > Cc: Helge Deller <deller@xxxxxx> > Cc: Hridya Valsaraju <hridya@xxxxxxxxxx> > Cc: James Morris <jamorris@xxxxxxxxxxxxxxxxxxx> > Cc: Jeff Vander Stoep <jeffv@xxxxxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx> > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> > Cc: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > Cc: Michal Hocko <mhocko@xxxxxxxx> > Cc: Michel Lespinasse <walken@xxxxxxxxxx> > Cc: Minchan Kim <minchan@xxxxxxxxxx> > Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> > Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx> > Cc: Szabolcs Nagy <szabolcs.nagy@xxxxxxx> > Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > --- > > fs/proc/base.c | 4 ++-- > fs/proc/fd.c | 15 ++++++++++++++- > 2 files changed, 16 insertions(+), 3 deletions(-) > > --- a/fs/proc/base.c~procfs-allow-reading-fdinfo-with-ptrace_mode_read > +++ a/fs/proc/base.c > @@ -3172,7 +3172,7 @@ static const struct pid_entry tgid_base_ > DIR("task", S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations), > DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations), > DIR("map_files", S_IRUSR|S_IXUSR, proc_map_files_inode_operations, proc_map_files_operations), > - DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations), > + DIR("fdinfo", S_IRUGO|S_IXUGO, proc_fdinfo_inode_operations, proc_fdinfo_operations), > DIR("ns", S_IRUSR|S_IXUGO, proc_ns_dir_inode_operations, proc_ns_dir_operations), > #ifdef CONFIG_NET > DIR("net", S_IRUGO|S_IXUGO, proc_net_inode_operations, proc_net_operations), > @@ -3517,7 +3517,7 @@ static const struct inode_operations pro > */ > static const struct pid_entry tid_base_stuff[] = { > DIR("fd", S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations), > - DIR("fdinfo", S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations), > + DIR("fdinfo", S_IRUGO|S_IXUGO, proc_fdinfo_inode_operations, proc_fdinfo_operations), > DIR("ns", S_IRUSR|S_IXUGO, proc_ns_dir_inode_operations, proc_ns_dir_operations), > #ifdef CONFIG_NET > DIR("net", S_IRUGO|S_IXUGO, proc_net_inode_operations, proc_net_operations), > --- a/fs/proc/fd.c~procfs-allow-reading-fdinfo-with-ptrace_mode_read > +++ a/fs/proc/fd.c > @@ -6,6 +6,7 @@ > #include <linux/fdtable.h> > #include <linux/namei.h> > #include <linux/pid.h> > +#include <linux/ptrace.h> > #include <linux/security.h> > #include <linux/file.h> > #include <linux/seq_file.h> > @@ -72,6 +73,18 @@ out: > > static int seq_fdinfo_open(struct inode *inode, struct file *file) > { > + bool allowed = false; > + struct task_struct *task = get_proc_task(inode); > + > + if (!task) > + return -ESRCH; > + > + allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS); > + put_task_struct(task); > + > + if (!allowed) > + return -EACCES; Uhm, this is only checked in open(), and never again? Is this safe in the face of exec or pid re-use? -Kees > + > return single_open(file, seq_show, inode); > } > > @@ -308,7 +321,7 @@ static struct dentry *proc_fdinfo_instan > struct proc_inode *ei; > struct inode *inode; > > - inode = proc_pid_make_inode(dentry->d_sb, task, S_IFREG | S_IRUSR); > + inode = proc_pid_make_inode(dentry->d_sb, task, S_IFREG | S_IRUGO); > if (!inode) > return ERR_PTR(-ENOENT); > > _ -- Kees Cook
