Messages in this thread | | | Subject | Re: [PATCH v1 3/7] kernel/fork: always deny write access to current MM exe_file | From | David Hildenbrand <> | Date | Thu, 12 Aug 2021 14:38:59 +0200 |
| |
On 12.08.21 14:32, Christian Brauner wrote: > On Thu, Aug 12, 2021 at 12:13:44PM +0200, David Hildenbrand wrote: >> On 12.08.21 12:05, Christian Brauner wrote: >>> [+Cc Andrei] >>> >>> On Thu, Aug 12, 2021 at 10:43:44AM +0200, David Hildenbrand wrote: >>>> We want to remove VM_DENYWRITE only currently only used when mapping the >>>> executable during exec. During exec, we already deny_write_access() the >>>> executable, however, after exec completes the VMAs mapped >>>> with VM_DENYWRITE effectively keeps write access denied via >>>> deny_write_access(). >>>> >>>> Let's deny write access when setting the MM exe_file. With this change, we >>>> can remove VM_DENYWRITE for mapping executables. >>>> >>>> This represents a minor user space visible change: >>>> sys_prctl(PR_SET_MM_EXE_FILE) can now fail if the file is already >>>> opened writable. Also, after sys_prctl(PR_SET_MM_EXE_FILE), the file >>> >>> Just for completeness, this also affects PR_SET_MM_MAP when exe_fd is >>> set. >> >> Correct. >> >>> >>>> cannot be opened writable. Note that we can already fail with -EACCES if >>>> the file doesn't have execute permissions. >>>> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>>> --- >>> >>> The biggest user I know and that I'm involved in is CRIU which heavily >>> uses PR_SET_MM_MAP (with a fallback to PR_SET_MM_EXE_FILE on older >>> kernels) during restore. Afair, criu opens the exe fd as an O_PATH >>> during dump and thus will use the same flag during restore when >>> opening it. So that should be fine. >> >> Yes. >> >>> >>> However, if I understand the consequences of this change correctly, a >>> problem could be restoring workloads that hold a writable fd open to >>> their exe file at dump time which would mean that during restore that fd >>> would be reopened writable causing CRIU to fail when setting the exe >>> file for the task to be restored. >> >> If it's their exe file, then the existing VM_DENYWRITE handling would have >> forbidden these workloads to open the fd of their exe file writable, right? > > Yes. > >> At least before doing any PR_SET_MM_MAP/PR_SET_MM_EXE_FILE. But that should >> rule out quite a lot of cases we might be worried about, right? > > Yes, it rules out the most obvious cases. The problem is really just > that we don't know how common weirder cases are. But that doesn't mean > we shouldn't try and risk it. This is a nice cleanup and playing > /proc/self/exe games isn't super common. >
Right, and having the file your executing opened writable isn't something very common as well.
If we really run into problems, we could not protect the new file when issuing PR_SET_MM_MAP/PR_SET_MM_EXE_FILE. But I'd like to avoid that, if possible, because it feels like working around something that never should have worked that way and is quite inconsistent.
-- Thanks,
David / dhildenb
| |