| Message ID | 20240712-vfs-procfs-ce7e6c7cf26b@brauner (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL,for,v6.11] vfs procfs | expand |
> 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
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
> But not this horror.
I agree and I didn't like it much myself (as evident from my pr message).
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(-)