diff options
| author | David Hildenbrand <david@redhat.com> | 2021-04-23 09:42:41 +0200 |
|---|---|---|
| committer | David Hildenbrand <david@redhat.com> | 2021-09-03 18:42:01 +0200 |
| commit | 4589ff7ca81516381393649dec8dd4948884b2b2 (patch) | |
| tree | 4bc5abab9bb7293f60d8b46dea87fb8e4097ae98 /fs/binfmt_elf.c | |
| parent | fe69d560b5bd9ec77b5d5749bd7027344daef47e (diff) | |
| download | lwn-4589ff7ca81516381393649dec8dd4948884b2b2.tar.gz lwn-4589ff7ca81516381393649dec8dd4948884b2b2.zip | |
binfmt: remove in-tree usage of MAP_DENYWRITE
At exec time when we mmap the new executable via MAP_DENYWRITE we have it
opened via do_open_execat() and already deny_write_access()'ed the file
successfully. Once exec completes, we allow_write_acces(); however,
we set mm->exe_file in begin_new_exec() via set_mm_exe_file() and
also deny_write_access() as long as mm->exe_file remains set. We'll
effectively deny write access to our executable via mm->exe_file
until mm->exe_file is changed -- when the process is removed, on new
exec, or via sys_prctl(PR_SET_MM_MAP/EXE_FILE).
Let's remove all usage of MAP_DENYWRITE, it's no longer necessary for
mm->exe_file.
In case of an elf interpreter, we'll now only deny write access to the file
during exec. This is somewhat okay, because the interpreter behaves
(and sometime is) a shared library; all shared libraries, especially the
ones loaded directly in user space like via dlopen() won't ever be mapped
via MAP_DENYWRITE, because we ignore that from user space completely;
these shared libraries can always be modified while mapped and executed.
Let's only special-case the main executable, denying write access while
being executed by a process. This can be considered a minor user space
visible change.
While this is a cleanup, it also fixes part of a problem reported with
VM_DENYWRITE on overlayfs, as VM_DENYWRITE is effectively unused with
this patch and will be removed next:
"Overlayfs did not honor positive i_writecount on realfile for
VM_DENYWRITE mappings." [1]
[1] https://lore.kernel.org/r/YNHXzBgzRrZu1MrD@miu.piliscsaba.redhat.com/
Reported-by: Chengguang Xu <cgxu519@mykernel.net>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Acked-by: Christian König <christian.koenig@amd.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Diffstat (limited to 'fs/binfmt_elf.c')
| -rw-r--r-- | fs/binfmt_elf.c | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 6d2c79533631..69d900a8473d 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -622,7 +622,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, eppnt = interp_elf_phdata; for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) { if (eppnt->p_type == PT_LOAD) { - int elf_type = MAP_PRIVATE | MAP_DENYWRITE; + int elf_type = MAP_PRIVATE; int elf_prot = make_prot(eppnt->p_flags, arch_state, true, true); unsigned long vaddr = 0; @@ -1070,7 +1070,7 @@ out_free_interp: elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, !!interpreter, false); - elf_flags = MAP_PRIVATE | MAP_DENYWRITE; + elf_flags = MAP_PRIVATE; vaddr = elf_ppnt->p_vaddr; /* |