mbox series

[GIT,PULL,for,v6.11] vfs procfs

Message ID 20240712-vfs-procfs-ce7e6c7cf26b@brauner (mailing list archive)
State New
Headers show
Series [GIT,PULL,for,v6.11] vfs procfs | expand

Pull-request

git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs tags/vfs-6.11.procfs

Message

Christian Brauner July 12, 2024, 1:58 p.m. UTC
Hey Linus,

/* Summary */
This contains work to allow restricting access to /proc/<pid>/mem. Over the
years various exploits started abusing /proc/<pid>/mem (cf. [1] and [2]).
Specifically [2] is interesting as it installed an arbitrary payload from
noexec storage into a running process and then executed it. That payload itself
included an ELF loader to run arbitrary code off of noexec storage.

The biggest problem is that /proc/<pid>/mem ignores page permissions by using
FOLL_FORCE which was discussed several times on- and off-list.

Unfortunately there are various use-cases using /proc/<pid>/mem making it
impossible to just turn it off. They at least include PTRACE_POKEDATA and the
seccomp notifier which is used to emulate system calls.

So give userspace a way to restrict access to /proc/<pid>/mem via kernel
command line options. Setting them to "all" restricts access for all processes
while "ptracer" will allow access to ptracers:

(1) Restrict the use of FOLL_FORCE via proc_mem.restrict_foll_force
(2) Restrict opening /proc/<pid>/mem for reading.
(3) Restrict opening /proc/<pid>/mem for writing.
(4) Restrict writing to /proc/<pid>/mem.

---

The level of fine-grained management isn't my favorite as it requires
distributions to have some level of knowledge around the implications of
FOLL_FORCE and /proc/<pid>/mem access in general. But the use-cases where
/proc/<pid>/mem access is needed do already imply a sophisticated knowledge
around its implications. Especially when it comes to the seccomp notifier and
gdb to inspect or emulate process state. So that ultimately swayed me to accept
this. If we need something simpler I'm all ears.

/* Testing */
clang: Debian clang version 16.0.6 (26)
gcc: (Debian 13.2.0-24)

All patches are based on v6.10-rc1 and have been sitting in linux-next.
No build failures or warnings were observed.

/* Conflicts */
No known conflicts.

The following changes since commit 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0:

  Linux 6.10-rc1 (2024-05-26 15:20:12 -0700)

are available in the Git repository at:

  git@gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs tags/vfs-6.11.procfs

for you to fetch changes up to 39efa92f9e5fceb44edef5536c58e3750b9d638d:

  proc: restrict /proc/pid/mem (2024-06-18 12:26:54 +0200)

Please consider pulling these changes from the signed vfs-6.11.procfs tag.

Thanks!
Christian

----------------------------------------------------------------
vfs-6.11.procfs

----------------------------------------------------------------
Adrian Ratiu (2):
      proc: pass file instead of inode to proc_mem_open
      proc: restrict /proc/pid/mem

 Documentation/admin-guide/kernel-parameters.txt |  38 +++++
 fs/proc/base.c                                  | 203 +++++++++++++++++++++++-
 fs/proc/internal.h                              |   2 +-
 fs/proc/task_mmu.c                              |   6 +-
 fs/proc/task_nommu.c                            |   2 +-
 security/Kconfig                                | 121 ++++++++++++++
 6 files changed, 363 insertions(+), 9 deletions(-)

Comments

Christian Brauner July 15, 2024, 11:17 a.m. UTC | #1
> years various exploits started abusing /proc/<pid>/mem (cf. [1] and [2]).
> Specifically [2] is interesting as it installed an arbitrary payload from

Sorry, the two links were missing:

[1]: https://lwn.net/Articles/476947
[2]: https://issues.chromium.org/issues/40089045
Linus Torvalds July 15, 2024, 7:20 p.m. UTC | #2
On Fri, 12 Jul 2024 at 06:59, Christian Brauner <brauner@kernel.org> wrote:
>
> The level of fine-grained management isn't my favorite as it requires
> distributions to have some level of knowledge around the implications of
> FOLL_FORCE and /proc/<pid>/mem access in general.

Ugh.

I pulled this, and looked at it, and then I decided I can't live with
something this ugly.

First off, there is ABSOLUTELY no reason for any of this to be using
static keys, which makes an already ugly patch even uglier. None of
this is magically so performance-critical that we'd need static keys
for this kind of thing

Secondly, this is absolutely the wrong kind of nairy rat's nest of
strange conditionals made worse by pointlessly adding kernel command
line options for it.

Now, the FOLL_FORCE is unquestionably problematic. But this horror
isn't making it better - it's just obfuscating a bad situation and
making it worse.

By all means just add one single kernel config option to say "no
FOLL_FORCE in /proc/pid/mem". Or require it to *actually* be traced,
or something like that.

But not this horror.

             Linus
Christian Brauner July 22, 2024, 2:49 p.m. UTC | #3
> But not this horror.

I agree and I didn't like it much myself (as evident from my pr message).