From: Kees Cook <keescook@chromium.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: Cyrill Gorcunov <gorcunov@gmail.com>, "Kirill A. Shutemov" <kirill@shutemov.name>, Calvin Owens <calvinowens@fb.com>, Alexey Dobriyan <adobriyan@gmail.com>, Oleg Nesterov <oleg@redhat.com>, "Eric W. Biederman" <ebiederm@xmission.com>, Al Viro <viro@zeniv.linux.org.uk>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Peter Feiner <pfeiner@google.com>, Grant Likely <grant.likely@secretlab.ca>, Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>, LKML <linux-kernel@vger.kernel.org>, kernel-team@fb.com, Pavel Emelyanov <xemul@openvz.org>, Linux API <linux-api@vger.kernel.org> Subject: Re: [RFC][PATCH v2] procfs: Always expose /proc/<pid>/map_files/ and make it readable Date: Mon, 26 Jan 2015 16:15:26 -0800 [thread overview] Message-ID: <CAGXu5jLDkg0hJSMm3CdoO-77yiK5GQWHSe3+1h7mq76LERpNBA@mail.gmail.com> (raw) In-Reply-To: <20150126154346.c63c512e5821e9e0ea31f759@linux-foundation.org> On Mon, Jan 26, 2015 at 3:43 PM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Tue, 27 Jan 2015 00:00:54 +0300 Cyrill Gorcunov <gorcunov@gmail.com> wrote: > >> On Mon, Jan 26, 2015 at 02:47:31PM +0200, Kirill A. Shutemov wrote: >> > On Fri, Jan 23, 2015 at 07:15:44PM -0800, Calvin Owens wrote: >> > > Currently, /proc/<pid>/map_files/ is restricted to CAP_SYS_ADMIN, and >> > > is only exposed if CONFIG_CHECKPOINT_RESTORE is set. This interface >> > > is very useful for enumerating the files mapped into a process when >> > > the more verbose information in /proc/<pid>/maps is not needed. > > This is the main (actually only) justification for the patch, and it it > far too thin. What does "not needed" mean. Why can't people just use > /proc/pid/maps? > >> > > This patch moves the folder out from behind CHECKPOINT_RESTORE, and >> > > removes the CAP_SYS_ADMIN restrictions. Following the links requires >> > > the ability to ptrace the process in question, so this doesn't allow >> > > an attacker to do anything they couldn't already do before. >> > > >> > > Signed-off-by: Calvin Owens <calvinowens@fb.com> >> > >> > Cc +linux-api@ >> >> Looks good to me, thanks! Though I would really appreciate if someone >> from security camp take a look as well. > > hm, who's that. Kees comes to mind. > > And reviewers' task would be a heck of a lot easier if they knew what > /proc/pid/map_files actually does. This: > > akpm3:/usr/src/25> grep -r map_files Documentation If akpm's comments weren't clear: this needs to be fixed. Everything in /proc should appear in Documentation. > akpm3:/usr/src/25> > > does not help. > > The 640708a2cff7f81 changelog says: > > : This one behaves similarly to the /proc/<pid>/fd/ one - it contains > : symlinks one for each mapping with file, the name of a symlink is > : "vma->vm_start-vma->vm_end", the target is the file. Opening a symlink > : results in a file that point exactly to the same inode as them vma's one. > : > : For example the ls -l of some arbitrary /proc/<pid>/map_files/ > : > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80403000-7f8f80404000 -> /lib64/libc-2.5.so > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f8061e000-7f8f80620000 -> /lib64/libselinux.so.1 > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80826000-7f8f80827000 -> /lib64/libacl.so.1.1.0 > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80a2f000-7f8f80a30000 -> /lib64/librt-2.5.so > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80a30000-7f8f80a4c000 -> /lib64/ld-2.5.so How is mmap offset represented in this output? > > afacit this info is also available in /proc/pid/maps, so things > shouldn't get worse if the /proc/pid/map_files permissions are at least > as restrictive as the /proc/pid/maps permissions. Is that the case? > (Please add to changelog). Both maps and map_files uses ptrace_may_access (via mm_acces) with PTRACE_MODE_READ, so I'm happy from a info leak perspective. Are mount namespaces handled in this output? > There's one other problem here: we're assuming that the map_files > implementation doesn't have bugs. If it does have bugs then relaxing > permissions like this will create new vulnerabilities. And the > map_files implementation is surprisingly complex. Is it bug-free? -Kees -- Kees Cook Chrome OS Security
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> To: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> Cc: Cyrill Gorcunov <gorcunov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Kirill A. Shutemov" <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>, Calvin Owens <calvinowens-b10kYP2dOMg@public.gmane.org>, Alexey Dobriyan <adobriyan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>, Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>, "Kirill A. Shutemov" <kirill.shutemov-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, Peter Feiner <pfeiner-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>, Siddhesh Poyarekar <siddhesh.poyarekar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, kernel-team-b10kYP2dOMg@public.gmane.org, Pavel Emelyanov <xemul-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>, Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [RFC][PATCH v2] procfs: Always expose /proc/<pid>/map_files/ and make it readable Date: Mon, 26 Jan 2015 16:15:26 -0800 [thread overview] Message-ID: <CAGXu5jLDkg0hJSMm3CdoO-77yiK5GQWHSe3+1h7mq76LERpNBA@mail.gmail.com> (raw) In-Reply-To: <20150126154346.c63c512e5821e9e0ea31f759-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> On Mon, Jan 26, 2015 at 3:43 PM, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote: > On Tue, 27 Jan 2015 00:00:54 +0300 Cyrill Gorcunov <gorcunov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > >> On Mon, Jan 26, 2015 at 02:47:31PM +0200, Kirill A. Shutemov wrote: >> > On Fri, Jan 23, 2015 at 07:15:44PM -0800, Calvin Owens wrote: >> > > Currently, /proc/<pid>/map_files/ is restricted to CAP_SYS_ADMIN, and >> > > is only exposed if CONFIG_CHECKPOINT_RESTORE is set. This interface >> > > is very useful for enumerating the files mapped into a process when >> > > the more verbose information in /proc/<pid>/maps is not needed. > > This is the main (actually only) justification for the patch, and it it > far too thin. What does "not needed" mean. Why can't people just use > /proc/pid/maps? > >> > > This patch moves the folder out from behind CHECKPOINT_RESTORE, and >> > > removes the CAP_SYS_ADMIN restrictions. Following the links requires >> > > the ability to ptrace the process in question, so this doesn't allow >> > > an attacker to do anything they couldn't already do before. >> > > >> > > Signed-off-by: Calvin Owens <calvinowens-b10kYP2dOMg@public.gmane.org> >> > >> > Cc +linux-api@ >> >> Looks good to me, thanks! Though I would really appreciate if someone >> from security camp take a look as well. > > hm, who's that. Kees comes to mind. > > And reviewers' task would be a heck of a lot easier if they knew what > /proc/pid/map_files actually does. This: > > akpm3:/usr/src/25> grep -r map_files Documentation If akpm's comments weren't clear: this needs to be fixed. Everything in /proc should appear in Documentation. > akpm3:/usr/src/25> > > does not help. > > The 640708a2cff7f81 changelog says: > > : This one behaves similarly to the /proc/<pid>/fd/ one - it contains > : symlinks one for each mapping with file, the name of a symlink is > : "vma->vm_start-vma->vm_end", the target is the file. Opening a symlink > : results in a file that point exactly to the same inode as them vma's one. > : > : For example the ls -l of some arbitrary /proc/<pid>/map_files/ > : > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80403000-7f8f80404000 -> /lib64/libc-2.5.so > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f8061e000-7f8f80620000 -> /lib64/libselinux.so.1 > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80826000-7f8f80827000 -> /lib64/libacl.so.1.1.0 > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80a2f000-7f8f80a30000 -> /lib64/librt-2.5.so > : | lr-x------ 1 root root 64 Aug 26 06:40 7f8f80a30000-7f8f80a4c000 -> /lib64/ld-2.5.so How is mmap offset represented in this output? > > afacit this info is also available in /proc/pid/maps, so things > shouldn't get worse if the /proc/pid/map_files permissions are at least > as restrictive as the /proc/pid/maps permissions. Is that the case? > (Please add to changelog). Both maps and map_files uses ptrace_may_access (via mm_acces) with PTRACE_MODE_READ, so I'm happy from a info leak perspective. Are mount namespaces handled in this output? > There's one other problem here: we're assuming that the map_files > implementation doesn't have bugs. If it does have bugs then relaxing > permissions like this will create new vulnerabilities. And the > map_files implementation is surprisingly complex. Is it bug-free? -Kees -- Kees Cook Chrome OS Security
next prev parent reply other threads:[~2015-01-27 0:15 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-14 0:20 [RFC][PATCH] procfs: Add /proc/<pid>/mapped_files Calvin Owens 2015-01-14 0:23 ` Calvin Owens 2015-01-14 14:13 ` Rasmus Villemoes 2015-01-14 14:37 ` Siddhesh Poyarekar 2015-01-14 14:53 ` Rasmus Villemoes 2015-01-14 21:03 ` Calvin Owens 2015-01-14 22:45 ` Andrew Morton 2015-01-14 23:51 ` Rasmus Villemoes 2015-01-16 1:15 ` Andrew Morton 2015-01-16 11:00 ` Kirill A. Shutemov 2015-01-14 15:25 ` Kirill A. Shutemov 2015-01-14 15:33 ` Cyrill Gorcunov 2015-01-14 20:46 ` Calvin Owens 2015-01-14 21:16 ` Cyrill Gorcunov 2015-01-22 2:45 ` [RFC][PATCH] procfs: Always expose /proc/<pid>/map_files/ and make it readable Calvin Owens 2015-01-22 7:16 ` Cyrill Gorcunov 2015-01-22 11:02 ` Kirill A. Shutemov 2015-01-22 21:00 ` Calvin Owens 2015-01-22 21:27 ` Kirill A. Shutemov 2015-01-23 5:52 ` Calvin Owens 2015-01-24 3:15 ` [RFC][PATCH v2] " Calvin Owens 2015-01-26 12:47 ` Kirill A. Shutemov 2015-01-26 21:00 ` Cyrill Gorcunov 2015-01-26 21:00 ` Cyrill Gorcunov 2015-01-26 23:43 ` Andrew Morton 2015-01-27 0:15 ` Kees Cook [this message] 2015-01-27 0:15 ` Kees Cook 2015-01-27 7:37 ` Cyrill Gorcunov 2015-01-27 7:37 ` Cyrill Gorcunov 2015-01-27 19:53 ` Kees Cook 2015-01-27 19:53 ` Kees Cook 2015-01-27 21:35 ` Cyrill Gorcunov 2015-01-27 21:35 ` Cyrill Gorcunov 2015-01-27 21:46 ` Pavel Emelyanov 2015-01-27 21:46 ` Pavel Emelyanov 2015-01-27 0:19 ` Kirill A. Shutemov 2015-01-27 0:19 ` Kirill A. Shutemov 2015-01-27 6:46 ` Cyrill Gorcunov 2015-01-27 6:46 ` Cyrill Gorcunov 2015-01-27 6:50 ` Andrew Morton 2015-01-27 7:23 ` Cyrill Gorcunov 2015-01-27 7:23 ` Cyrill Gorcunov 2015-01-28 4:38 ` Calvin Owens 2015-01-28 4:38 ` Calvin Owens 2015-01-30 1:30 ` Kees Cook 2015-01-30 1:30 ` Kees Cook 2015-01-31 1:58 ` Calvin Owens 2015-01-31 1:58 ` Calvin Owens 2015-02-02 14:01 ` Austin S Hemmelgarn 2015-02-04 3:53 ` Calvin Owens 2015-02-04 3:53 ` Calvin Owens 2015-02-02 20:16 ` Andy Lutomirski 2015-02-04 3:28 ` Calvin Owens 2015-02-04 3:28 ` Calvin Owens 2015-02-12 2:29 ` [RFC][PATCH v3] " Calvin Owens 2015-02-12 7:45 ` Cyrill Gorcunov 2015-02-14 20:40 ` [RFC][PATCH v4] " Calvin Owens 2015-03-10 22:17 ` Cyrill Gorcunov 2015-04-28 22:23 ` Calvin Owens 2015-04-29 7:32 ` Cyrill Gorcunov 2015-05-19 3:10 ` [PATCH v5] " Calvin Owens 2015-05-19 3:29 ` Joe Perches 2015-05-19 18:04 ` Andy Lutomirski 2015-05-21 1:52 ` Calvin Owens 2015-05-21 2:10 ` Andy Lutomirski 2015-06-09 3:39 ` [PATCH v6] " Calvin Owens 2015-06-09 17:27 ` Kees Cook 2015-06-09 17:47 ` Andy Lutomirski 2015-06-09 18:15 ` Cyrill Gorcunov 2015-06-09 21:13 ` Andrew Morton 2015-06-10 1:39 ` Calvin Owens 2015-06-10 20:58 ` Andrew Morton 2015-06-11 11:10 ` Alexey Dobriyan 2015-06-11 18:49 ` Andrew Morton 2015-06-12 9:55 ` Alexey Dobriyan 2015-06-19 2:32 ` [PATCH v7] " Calvin Owens 2015-07-15 22:21 ` Andrew Morton 2015-07-15 23:39 ` Calvin Owens 2015-02-14 20:44 ` [PATCH] procfs: Return -ESRCH on /proc/N/fd/* when PID N doesn't exist Calvin Owens 2015-01-14 22:40 ` [RFC][PATCH] procfs: Add /proc/<pid>/mapped_files Kirill A. Shutemov
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=CAGXu5jLDkg0hJSMm3CdoO-77yiK5GQWHSe3+1h7mq76LERpNBA@mail.gmail.com \ --to=keescook@chromium.org \ --cc=adobriyan@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=calvinowens@fb.com \ --cc=ebiederm@xmission.com \ --cc=gorcunov@gmail.com \ --cc=grant.likely@secretlab.ca \ --cc=kernel-team@fb.com \ --cc=kirill.shutemov@linux.intel.com \ --cc=kirill@shutemov.name \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=oleg@redhat.com \ --cc=pfeiner@google.com \ --cc=siddhesh.poyarekar@gmail.com \ --cc=viro@zeniv.linux.org.uk \ --cc=xemul@openvz.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.