From: "Christian König" <christian.koenig@amd.com>
To: Kalesh Singh <kaleshsingh@google.com>
Cc: jannh@google.com, jeffv@google.com, keescook@chromium.org,
surenb@google.com, minchan@kernel.org, hridya@google.com,
rdunlap@infradead.org, willy@infradead.org,
viro@zeniv.linux.org.uk, kernel-team@android.com,
Alexey Dobriyan <adobriyan@gmail.com>,
Jonathan Corbet <corbet@lwn.net>,
Andrew Morton <akpm@linux-foundation.org>,
Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
Michal Hocko <mhocko@suse.com>,
Alexey Gladkov <gladkov.alexey@gmail.com>,
Szabolcs Nagy <szabolcs.nagy@arm.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Christian Brauner <christian.brauner@ubuntu.com>,
Michel Lespinasse <walken@google.com>,
Bernd Edlinger <bernd.edlinger@hotmail.de>,
Andrei Vagin <avagin@gmail.com>, Helge Deller <deller@gmx.de>,
James Morris <jamorris@linux.microsoft.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-doc@vger.kernel.org
Subject: Re: [RESEND PATCH v6 1/2] procfs: Allow reading fdinfo with PTRACE_MODE_READ
Date: Mon, 8 Mar 2021 18:54:13 +0100 [thread overview]
Message-ID: <a51dfd94-185a-63f1-3dba-84dcbe94cb56@amd.com> (raw)
In-Reply-To: <20210308170651.919148-1-kaleshsingh@google.com>
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@google.com>
> Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
Both patches are Acked-by: Christian König <christian.koenig@amd.com>
> ---
> 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);
>
next prev parent reply other threads:[~2021-03-08 17:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-08 17:06 [RESEND PATCH v6 1/2] procfs: Allow reading fdinfo with PTRACE_MODE_READ Kalesh Singh
2021-03-08 17:06 ` [RESEND PATCH v6 2/2] procfs/dmabuf: Add inode number to /proc/*/fdinfo Kalesh Singh
2021-03-18 5:54 ` Kees Cook
2021-03-08 17:54 ` Christian König [this message]
2021-03-08 18:06 ` [RESEND PATCH v6 1/2] procfs: Allow reading fdinfo with PTRACE_MODE_READ Kalesh Singh
2021-03-18 5:55 ` Kees Cook
2021-03-18 6:14 ` Kalesh Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a51dfd94-185a-63f1-3dba-84dcbe94cb56@amd.com \
--to=christian.koenig@amd.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=avagin@gmail.com \
--cc=bernd.edlinger@hotmail.de \
--cc=christian.brauner@ubuntu.com \
--cc=corbet@lwn.net \
--cc=deller@gmx.de \
--cc=ebiederm@xmission.com \
--cc=gladkov.alexey@gmail.com \
--cc=hridya@google.com \
--cc=jamorris@linux.microsoft.com \
--cc=jannh@google.com \
--cc=jeffv@google.com \
--cc=kaleshsingh@google.com \
--cc=keescook@chromium.org \
--cc=kernel-team@android.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab+huawei@kernel.org \
--cc=mhocko@suse.com \
--cc=minchan@kernel.org \
--cc=rdunlap@infradead.org \
--cc=surenb@google.com \
--cc=szabolcs.nagy@arm.com \
--cc=viro@zeniv.linux.org.uk \
--cc=walken@google.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).