From: Daniel Colascione <dancol@google.com> To: Kees Cook <keescook@chromium.org> Cc: Tim Murray <timmurray@google.com>, Nosh Minwalla <nosh@google.com>, Nick Kralevich <nnk@google.com>, Lokesh Gidra <lokeshgidra@google.com>, linux-kernel <linux-kernel@vger.kernel.org>, Linux API <linux-api@vger.kernel.org>, selinux@vger.kernel.org, Andrea Arcangeli <aarcange@redhat.com>, Mike Rapoport <rppt@linux.ibm.com>, Peter Xu <peterx@redhat.com>, Jann Horn <jannh@google.com>, LSM List <linux-security-module@vger.kernel.org> Subject: Re: [PATCH v2 0/6] Harden userfaultfd Date: Wed, 12 Feb 2020 09:12:00 -0800 [thread overview] Message-ID: <CAKOZuesS1_O0OuePwSN2Kc4b-vyokiV=MvBd4nDi=umBKM6v6w@mail.gmail.com> (raw) In-Reply-To: <202002112332.BE71455@keescook> Thanks for taking a look and for the fast reply! On Tue, Feb 11, 2020 at 11:51 PM Kees Cook <keescook@chromium.org> wrote: > > Hi! > > Firstly, thanks for working on this! It's been on my TODO list for a > while. :) > > Casey already recommended including the LSM list to CC (since this is a > new LSM -- there are many LSMs). Additionally, the series should > probably be sent _to_ the userfaultfd maintainers: > Andrea Arcangeli <aarcange@redhat.com> > Mike Rapoport <rppt@linux.ibm.com> > and I'd also CC a couple other people that have done recent work: > Peter Xu <peterx@redhat.com> > Jann Horn <jannh@google.com> > > More notes below... In general, in the event that a patch series doesn't include all needed parties on the to-line, what's the right way to fix the situation without spamming everyone and forking the thread? In this case, since I'm splitting the patch series anyway, I can just expand the to-line in the reroll. > On Tue, Feb 11, 2020 at 02:55:41PM -0800, Daniel Colascione wrote: > > Userfaultfd in unprivileged contexts could be potentially very > > useful. We'd like to harden userfaultfd to make such unprivileged use > > less risky. This patch series allows SELinux to manage userfaultfd > > file descriptors and allows administrators to limit userfaultfd to > > servicing user-mode faults, increasing the difficulty of using > > userfaultfd in exploit chains invoking delaying kernel faults. > > I actually think these are two very different goals and likely the > series could be split into two for them. One is LSM hooking of > userfaultfd and the SELinux attachment, and the other is the user-mode > fault restrictions. And they would likely go via separate trees (LSM > through James's LSM tree, and probably akpm's -mm tree for the sysctl). > > > A new anon_inodes interface allows callers to opt into SELinux > > management of anonymous file objects. In this mode, anon_inodes > > creates new ephemeral inodes for anonymous file objects instead of > > reusing a singleton dummy inode. A new LSM hook gives security modules > > an opportunity to configure and veto these ephemeral inodes. > > > > Existing anon_inodes users must opt into the new functionality. > > > > Daniel Colascione (6): > > Add a new flags-accepting interface for anonymous inodes > > Add a concept of a "secure" anonymous file > > Teach SELinux about a new userfaultfd class > > Wire UFFD up to SELinux > > The above is the first "series"... I don't have much opinion about it, > though I do like the idea of making userfaultfd visible to the LSM. Yeah. The interesting part there is the anon_inodes API change. I'll split that half of the series out. > > Let userfaultfd opt out of handling kernel-mode faults > > Add a new sysctl for limiting userfaultfd to user mode faults > > Now this I'm very interested in. Can you go into more detail about two > things: > > - What is the threat being solved? (I understand the threat, but detailing > it in the commit log is important for people who don't know it. Existing > commit cefdca0a86be517bc390fc4541e3674b8e7803b0 gets into some of the > details already, but I'd like to see reference to external sources like > https://duasynt.com/blog/linux-kernel-heap-spray) Sure. I can add a reference to that and a more general discussion of how delaying kernel fault handling can broaden race windows for other exploits. > - Why is this needed in addition to the existing vm.unprivileged_userfaultfd > sysctl? (And should this maybe just be another setting for that > sysctl, like "2"?) We want to use UFFD for a new garbage collector in Android, and we want unprivileged processes to be able to use this new garbage collector. Giving them CAP_SYS_PTRACE is dangerous. > As to the mechanics of the change, I'm not sure I like the idea of adding > a UAPI flag for this. Why not just retain the permission check done at > open() and if kernelmode faults aren't allowed, ignore them? This would > require no changes to existing programs and gains the desired defense. As Jann mentions below, it's a matter of the kernel's contractual obligation to userspace. Right now, userfaultfd(2) either succeeds or it fails with one of the enumerated error codes. You're proposing having the userfaultfd(2) succeed but return a file descriptor that doesn't do the job the kernel promised it would do. If we were to adopt your proposal, an application would see UFFD succeed, then see unexpeced EFAULTs from system calls, which would probably cause the application to malfunctioning in exciting ways. An explicit "sorry, I can't do that" error code is better: an application that gets an error code from userfaultfd(2) can fall back to something else, but an application that silently gets an underfeatured UFFD doesn't know anything is wrong until it's too late. The additional flag preserves the UFFD contract and gives applications a way to probe for feature support. > (And, I think, the sysctl value could be bumped to "2" as that's a > better default state -- does qemu actually need kernelmode traps?) I prefer a default of one for regular systems because I don't want to make experimentation with novel ways to use UFFD more difficult, and a default of two would require users go out of their way to handle user faults, and few will. For a more constrained system like Android, two is fine.
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Colascione <dancol-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> To: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Cc: Tim Murray <timmurray-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Nosh Minwalla <nosh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Nick Kralevich <nnk-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Lokesh Gidra <lokeshgidra-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, linux-kernel <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Linux API <linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, selinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org>, Peter Xu <peterx-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Jann Horn <jannh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, LSM List <linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH v2 0/6] Harden userfaultfd Date: Wed, 12 Feb 2020 09:12:00 -0800 [thread overview] Message-ID: <CAKOZuesS1_O0OuePwSN2Kc4b-vyokiV=MvBd4nDi=umBKM6v6w@mail.gmail.com> (raw) In-Reply-To: <202002112332.BE71455@keescook> Thanks for taking a look and for the fast reply! On Tue, Feb 11, 2020 at 11:51 PM Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > > Hi! > > Firstly, thanks for working on this! It's been on my TODO list for a > while. :) > > Casey already recommended including the LSM list to CC (since this is a > new LSM -- there are many LSMs). Additionally, the series should > probably be sent _to_ the userfaultfd maintainers: > Andrea Arcangeli <aarcange-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Mike Rapoport <rppt-tEXmvtCZX7AybS5Ee8rs3A@public.gmane.org> > and I'd also CC a couple other people that have done recent work: > Peter Xu <peterx-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Jann Horn <jannh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > > More notes below... In general, in the event that a patch series doesn't include all needed parties on the to-line, what's the right way to fix the situation without spamming everyone and forking the thread? In this case, since I'm splitting the patch series anyway, I can just expand the to-line in the reroll. > On Tue, Feb 11, 2020 at 02:55:41PM -0800, Daniel Colascione wrote: > > Userfaultfd in unprivileged contexts could be potentially very > > useful. We'd like to harden userfaultfd to make such unprivileged use > > less risky. This patch series allows SELinux to manage userfaultfd > > file descriptors and allows administrators to limit userfaultfd to > > servicing user-mode faults, increasing the difficulty of using > > userfaultfd in exploit chains invoking delaying kernel faults. > > I actually think these are two very different goals and likely the > series could be split into two for them. One is LSM hooking of > userfaultfd and the SELinux attachment, and the other is the user-mode > fault restrictions. And they would likely go via separate trees (LSM > through James's LSM tree, and probably akpm's -mm tree for the sysctl). > > > A new anon_inodes interface allows callers to opt into SELinux > > management of anonymous file objects. In this mode, anon_inodes > > creates new ephemeral inodes for anonymous file objects instead of > > reusing a singleton dummy inode. A new LSM hook gives security modules > > an opportunity to configure and veto these ephemeral inodes. > > > > Existing anon_inodes users must opt into the new functionality. > > > > Daniel Colascione (6): > > Add a new flags-accepting interface for anonymous inodes > > Add a concept of a "secure" anonymous file > > Teach SELinux about a new userfaultfd class > > Wire UFFD up to SELinux > > The above is the first "series"... I don't have much opinion about it, > though I do like the idea of making userfaultfd visible to the LSM. Yeah. The interesting part there is the anon_inodes API change. I'll split that half of the series out. > > Let userfaultfd opt out of handling kernel-mode faults > > Add a new sysctl for limiting userfaultfd to user mode faults > > Now this I'm very interested in. Can you go into more detail about two > things: > > - What is the threat being solved? (I understand the threat, but detailing > it in the commit log is important for people who don't know it. Existing > commit cefdca0a86be517bc390fc4541e3674b8e7803b0 gets into some of the > details already, but I'd like to see reference to external sources like > https://duasynt.com/blog/linux-kernel-heap-spray) Sure. I can add a reference to that and a more general discussion of how delaying kernel fault handling can broaden race windows for other exploits. > - Why is this needed in addition to the existing vm.unprivileged_userfaultfd > sysctl? (And should this maybe just be another setting for that > sysctl, like "2"?) We want to use UFFD for a new garbage collector in Android, and we want unprivileged processes to be able to use this new garbage collector. Giving them CAP_SYS_PTRACE is dangerous. > As to the mechanics of the change, I'm not sure I like the idea of adding > a UAPI flag for this. Why not just retain the permission check done at > open() and if kernelmode faults aren't allowed, ignore them? This would > require no changes to existing programs and gains the desired defense. As Jann mentions below, it's a matter of the kernel's contractual obligation to userspace. Right now, userfaultfd(2) either succeeds or it fails with one of the enumerated error codes. You're proposing having the userfaultfd(2) succeed but return a file descriptor that doesn't do the job the kernel promised it would do. If we were to adopt your proposal, an application would see UFFD succeed, then see unexpeced EFAULTs from system calls, which would probably cause the application to malfunctioning in exciting ways. An explicit "sorry, I can't do that" error code is better: an application that gets an error code from userfaultfd(2) can fall back to something else, but an application that silently gets an underfeatured UFFD doesn't know anything is wrong until it's too late. The additional flag preserves the UFFD contract and gives applications a way to probe for feature support. > (And, I think, the sysctl value could be bumped to "2" as that's a > better default state -- does qemu actually need kernelmode traps?) I prefer a default of one for regular systems because I don't want to make experimentation with novel ways to use UFFD more difficult, and a default of two would require users go out of their way to handle user faults, and few will. For a more constrained system like Android, two is fine.
next prev parent reply other threads:[~2020-02-12 17:12 UTC|newest] Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-11 22:55 [PATCH v2 0/6] Harden userfaultfd Daniel Colascione 2020-02-11 22:55 ` Daniel Colascione 2020-02-11 22:55 ` [PATCH v2 1/6] Add a new flags-accepting interface for anonymous inodes Daniel Colascione 2020-02-12 16:37 ` Stephen Smalley 2020-02-12 17:23 ` Daniel Colascione 2020-02-12 17:23 ` Daniel Colascione 2020-02-11 22:55 ` [PATCH v2 2/6] Add a concept of a "secure" anonymous file Daniel Colascione 2020-02-12 16:49 ` Stephen Smalley 2020-02-14 22:13 ` kbuild test robot 2020-02-14 22:13 ` kbuild test robot 2020-02-11 22:55 ` [PATCH v2 3/6] Teach SELinux about a new userfaultfd class Daniel Colascione 2020-02-12 17:05 ` Stephen Smalley 2020-02-12 17:19 ` Daniel Colascione 2020-02-12 18:04 ` Stephen Smalley 2020-02-12 18:04 ` Stephen Smalley 2020-02-12 18:59 ` Stephen Smalley 2020-02-12 19:04 ` Daniel Colascione 2020-02-12 19:04 ` Daniel Colascione 2020-02-12 19:11 ` Stephen Smalley 2020-02-12 19:11 ` Stephen Smalley 2020-02-12 19:13 ` Daniel Colascione 2020-02-12 19:13 ` Daniel Colascione 2020-02-12 19:17 ` Stephen Smalley 2020-02-11 22:55 ` [PATCH v2 4/6] Wire UFFD up to SELinux Daniel Colascione 2020-02-11 22:55 ` Daniel Colascione 2020-02-11 22:55 ` [PATCH v2 5/6] Let userfaultfd opt out of handling kernel-mode faults Daniel Colascione 2020-02-11 22:55 ` Daniel Colascione 2020-02-11 22:55 ` [PATCH v2 6/6] Add a new sysctl for limiting userfaultfd to user mode faults Daniel Colascione 2020-02-11 23:13 ` [PATCH v2 0/6] Harden userfaultfd Casey Schaufler 2020-02-11 23:13 ` Casey Schaufler 2020-02-11 23:27 ` Daniel Colascione 2020-02-12 16:09 ` Stephen Smalley 2020-02-21 17:56 ` James Morris 2020-02-12 7:50 ` Kees Cook 2020-02-12 7:50 ` Kees Cook 2020-02-12 16:54 ` Jann Horn 2020-02-12 16:54 ` Jann Horn 2020-02-12 17:14 ` Peter Xu 2020-02-12 19:41 ` Andrea Arcangeli 2020-02-12 19:41 ` Andrea Arcangeli 2020-02-12 20:04 ` Daniel Colascione 2020-02-12 23:41 ` Andrea Arcangeli 2020-02-12 17:12 ` Daniel Colascione [this message] 2020-02-12 17:12 ` Daniel Colascione 2020-02-14 3:26 ` [PATCH 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-02-14 3:26 ` [PATCH 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione 2020-02-14 3:26 ` [PATCH 2/3] Teach SELinux about anonymous inodes Daniel Colascione 2020-02-14 16:39 ` Stephen Smalley 2020-02-14 17:21 ` Daniel Colascione 2020-02-14 18:02 ` Stephen Smalley 2020-02-14 18:08 ` Stephen Smalley 2020-02-14 20:24 ` Stephen Smalley 2020-02-14 3:26 ` [PATCH 3/3] Wire UFFD up to SELinux Daniel Colascione 2020-03-25 23:02 ` [PATCH v2 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-03-25 23:02 ` [PATCH v2 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione 2020-03-26 13:53 ` Stephen Smalley 2020-03-25 23:02 ` [PATCH v2 2/3] Teach SELinux about anonymous inodes Daniel Colascione 2020-03-26 13:58 ` Stephen Smalley 2020-03-26 17:59 ` Daniel Colascione 2020-03-26 17:37 ` Stephen Smalley 2020-03-25 23:02 ` [PATCH v2 3/3] Wire UFFD up to SELinux Daniel Colascione 2020-03-25 23:49 ` Casey Schaufler 2020-03-26 18:14 ` [PATCH v3 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-03-26 18:14 ` [PATCH v3 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione 2020-03-26 19:00 ` Stephen Smalley 2020-03-26 18:14 ` [PATCH v3 2/3] Teach SELinux about anonymous inodes Daniel Colascione 2020-03-26 19:02 ` Stephen Smalley 2020-03-26 18:14 ` [PATCH v3 3/3] Wire UFFD up to SELinux Daniel Colascione 2020-03-26 20:06 ` [PATCH v4 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-03-26 20:06 ` [PATCH v4 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione 2020-03-27 13:40 ` Stephen Smalley 2020-03-26 20:06 ` [PATCH v4 2/3] Teach SELinux about anonymous inodes Daniel Colascione 2020-03-27 13:41 ` Stephen Smalley 2020-03-26 20:06 ` [PATCH v4 3/3] Wire UFFD up to SELinux Daniel Colascione 2020-04-01 21:39 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-04-01 21:39 ` [PATCH v5 1/3] Add a new LSM-supporting anonymous inode interface Daniel Colascione 2020-05-07 16:02 ` James Morris 2020-08-04 21:22 ` Eric Biggers 2020-04-01 21:39 ` [PATCH v5 2/3] Teach SELinux about anonymous inodes Daniel Colascione 2020-04-01 21:39 ` [PATCH v5 3/3] Wire UFFD up to SELinux Daniel Colascione 2020-08-04 21:16 ` Eric Biggers 2020-04-13 13:29 ` [PATCH v5 0/3] SELinux support for anonymous inodes and UFFD Daniel Colascione 2020-04-22 16:55 ` James Morris 2020-04-22 17:12 ` Casey Schaufler 2020-04-23 22:24 ` Casey Schaufler 2020-04-27 16:18 ` Casey Schaufler 2020-04-27 16:48 ` Stephen Smalley 2020-04-27 17:12 ` Casey Schaufler 2020-04-29 17:02 ` Stephen Smalley 2020-04-27 17:15 ` Casey Schaufler 2020-04-27 19:40 ` Stephen Smalley 2020-06-04 3:56 ` James Morris 2020-06-04 18:51 ` Stephen Smalley 2020-06-04 19:24 ` Lokesh Gidra
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='CAKOZuesS1_O0OuePwSN2Kc4b-vyokiV=MvBd4nDi=umBKM6v6w@mail.gmail.com' \ --to=dancol@google.com \ --cc=aarcange@redhat.com \ --cc=jannh@google.com \ --cc=keescook@chromium.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=lokeshgidra@google.com \ --cc=nnk@google.com \ --cc=nosh@google.com \ --cc=peterx@redhat.com \ --cc=rppt@linux.ibm.com \ --cc=selinux@vger.kernel.org \ --cc=timmurray@google.com \ /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.