| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAC_TJve-vJPHfc7k-JuueoyCkKskv6ThVkrXzDA0rX85M4A82g@mail.gmail.com>
Date: Mon, 22 Feb 2021 18:55:59 -0500
From: Kalesh Singh <kaleshsingh@...gle.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Jann Horn <jannh@...gle.com>,
Jeffrey Vander Stoep <jeffv@...gle.com>,
Kees Cook <keescook@...omium.org>,
Suren Baghdasaryan <surenb@...gle.com>,
Minchan Kim <minchan@...nel.org>,
Hridya Valsaraju <hridya@...gle.com>,
Randy Dunlap <rdunlap@...radead.org>,
Christian König <christian.koenig@....com>,
Matthew Wilcox <willy@...radead.org>,
"Cc: Android Kernel" <kernel-team@...roid.com>,
Alexey Dobriyan <adobriyan@...il.com>,
Jonathan Corbet <corbet@....net>,
Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Michal Hocko <mhocko@...e.com>,
Alexey Gladkov <gladkov.alexey@...il.com>,
NeilBrown <neilb@...e.de>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Daniel Jordan <daniel.m.jordan@...cle.com>,
Michel Lespinasse <walken@...gle.com>,
Bernd Edlinger <bernd.edlinger@...mail.de>,
Andrei Vagin <avagin@...il.com>,
Yafang Shao <laoar.shao@...il.com>,
Christian Brauner <christian.brauner@...ntu.com>,
LKML <linux-kernel@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v6 1/2] procfs: Allow reading fdinfo with PTRACE_MODE_READ
On Mon, Feb 8, 2021 at 10:53 AM Kalesh Singh <kaleshsingh@...gle.com> wrote:
>
> 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@...gle.com>
> Signed-off-by: Kalesh Singh <kaleshsingh@...gle.com>
> ---
> Changes in v2:
> - Update patch description
Hi all,
Kindly requesting maintainers to take a look at this patch set.
Thanks,
Kalesh
>
> 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 b3422cda2a91..a37f9de7103f 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -3160,7 +3160,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 cb51763ed554..585e213301f9 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);
> }
>
> @@ -307,7 +320,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);
>
> --
> 2.30.0.478.g8a0d178c01-goog
>
Powered by blists - more mailing lists