On Mon, Mar 8, 2021 at 12:54 PM Christian König <christian.koenig@xxxxxxx> wrote: > > Am 08.03.21 um 18:06 schrieb Kalesh Singh: > > 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. > > > > Suggested-by: Jann Horn <jannh@xxxxxxxxxx> > > Signed-off-by: Kalesh Singh <kaleshsingh@xxxxxxxxxx> > > Both patches are Acked-by: Christian König <christian.koenig@xxxxxxx> Thanks Christian. > > > --- > > Hi everyone, > > > > The initial posting of this patch can be found at [1]. > > I didn't receive any feedback last time, so resending here. > > Would really appreciate any constructive comments/suggestions. > > > > Thanks, > > Kalesh > > > > [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20210208155315.1367371-1-kaleshsingh%40google.com%2F&data=04%7C01%7Cchristian.koenig%40amd.com%7C38c98420f0564e15117f08d8e2549ff5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637508200431130855%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=deJBlAk6%2BEQkfAC8iRK95xhV1%2FiO9Si%2Bylc5Z0QzzrM%3D&reserved=0 > > > > Changes in v2: > > - Update patch description > > fs/proc/base.c | 4 ++-- > > fs/proc/fd.c | 15 ++++++++++++++- > > 2 files changed, 16 insertions(+), 3 deletions(-) > > > > diff --git a/fs/proc/base.c b/fs/proc/base.c > > index 3851bfcdba56..fd46d8dd0cf4 100644 > > --- a/fs/proc/base.c > > +++ b/fs/proc/base.c > > @@ -3159,7 +3159,7 @@ static const struct pid_entry tgid_base_stuff[] = { > > 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), > > @@ -3504,7 +3504,7 @@ static const struct inode_operations proc_tid_comm_inode_operations = { > > */ > > 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), > > diff --git a/fs/proc/fd.c b/fs/proc/fd.c > > index 07fc4fad2602..6a80b40fd2fe 100644 > > --- a/fs/proc/fd.c > > +++ b/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 @@ static int seq_show(struct seq_file *m, void *v) > > > > 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; > > + > > return single_open(file, seq_show, inode); > > } > > > > @@ -308,7 +321,7 @@ static struct dentry *proc_fdinfo_instantiate(struct dentry *dentry, > > 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); > > > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@xxxxxxxxxxx. >
