Skip to content

Commit

Permalink
Merge tag 'denywrite-for-5.15' of git://github.com/davidhildenbrand/l…
Browse files Browse the repository at this point in the history
…inux

Pull MAP_DENYWRITE removal from David Hildenbrand:
 "Remove all in-tree usage of MAP_DENYWRITE from the kernel and remove
  VM_DENYWRITE.

  There are some (minor) user-visible changes:

   - We no longer deny write access to shared libaries loaded via legacy
     uselib(); this behavior matches modern user space e.g. dlopen().

   - We no longer deny write access to the elf interpreter after exec
     completed, treating it just like shared libraries (which it often
     is).

   - We always deny write access to the file linked via /proc/pid/exe:
     sys_prctl(PR_SET_MM_MAP/EXE_FILE) will fail if write access to the
     file cannot be denied, and write access to the file will remain
     denied until the link is effectivel gone (exec, termination,
     sys_prctl(PR_SET_MM_MAP/EXE_FILE)) -- just as if exec'ing the file.

  Cross-compiled for a bunch of architectures (alpha, microblaze, i386,
  s390x, ...) and verified via ltp that especially the relevant tests
  (i.e., creat07 and execve04) continue working as expected"

* tag 'denywrite-for-5.15' of git://github.com/davidhildenbrand/linux:
  fs: update documentation of get_write_access() and friends
  mm: ignore MAP_DENYWRITE in ksys_mmap_pgoff()
  mm: remove VM_DENYWRITE
  binfmt: remove in-tree usage of MAP_DENYWRITE
  kernel/fork: always deny write access to current MM exe_file
  kernel/fork: factor out replacing the current MM exe_file
  binfmt: don't use MAP_DENYWRITE when loading shared libraries via uselib()
  • Loading branch information
torvalds committed Sep 4, 2021
2 parents f746406 + 592ca09 commit 49624ef
Show file tree
Hide file tree
Showing 16 changed files with 119 additions and 103 deletions.
8 changes: 3 additions & 5 deletions arch/x86/ia32/ia32_aout.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,17 +202,15 @@ static int load_aout_binary(struct linux_binprm *bprm)

error = vm_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
PROT_READ | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE |
MAP_32BIT,
MAP_FIXED | MAP_PRIVATE | MAP_32BIT,
fd_offset);

if (error != N_TXTADDR(ex))
return error;

error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE |
MAP_32BIT,
MAP_FIXED | MAP_PRIVATE | MAP_32BIT,
fd_offset + ex.a_text);
if (error != N_DATADDR(ex))
return error;
Expand Down Expand Up @@ -293,7 +291,7 @@ static int load_aout_library(struct file *file)
/* Now use mmap to map the library into memory. */
error = vm_mmap(file, start_addr, ex.a_text + ex.a_data,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_32BIT,
MAP_FIXED | MAP_PRIVATE | MAP_32BIT,
N_TXTOFF(ex));
retval = error;
if (error != start_addr)
Expand Down
7 changes: 3 additions & 4 deletions fs/binfmt_aout.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,15 @@ static int load_aout_binary(struct linux_binprm * bprm)
}

error = vm_mmap(bprm->file, N_TXTADDR(ex), ex.a_text,
PROT_READ | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
PROT_READ | PROT_EXEC, MAP_FIXED | MAP_PRIVATE,
fd_offset);

if (error != N_TXTADDR(ex))
return error;

error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
MAP_FIXED | MAP_PRIVATE,
fd_offset + ex.a_text);
if (error != N_DATADDR(ex))
return error;
Expand Down Expand Up @@ -309,7 +308,7 @@ static int load_aout_library(struct file *file)
/* Now use mmap to map the library into memory. */
error = vm_mmap(file, start_addr, ex.a_text + ex.a_data,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
MAP_FIXED | MAP_PRIVATE;
N_TXTOFF(ex));
retval = error;
if (error != start_addr)
Expand Down
6 changes: 3 additions & 3 deletions fs/binfmt_elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1070,7 +1070,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
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;
/*
Expand Down Expand Up @@ -1384,7 +1384,7 @@ static int load_elf_library(struct file *file)
(eppnt->p_filesz +
ELF_PAGEOFFSET(eppnt->p_vaddr)),
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED_NOREPLACE | MAP_PRIVATE | MAP_DENYWRITE,
MAP_FIXED_NOREPLACE | MAP_PRIVATE,
(eppnt->p_offset -
ELF_PAGEOFFSET(eppnt->p_vaddr)));
if (error != ELF_PAGESTART(eppnt->p_vaddr))
Expand Down
2 changes: 1 addition & 1 deletion fs/binfmt_elf_fdpic.c
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *params,
if (phdr->p_flags & PF_W) prot |= PROT_WRITE;
if (phdr->p_flags & PF_X) prot |= PROT_EXEC;

flags = MAP_PRIVATE | MAP_DENYWRITE;
flags = MAP_PRIVATE;
maddr = 0;

switch (params->flags & ELF_FDPIC_FLAG_ARRANGEMENT) {
Expand Down
4 changes: 3 additions & 1 deletion fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,9 @@ int begin_new_exec(struct linux_binprm * bprm)
* not visibile until then. This also enables the update
* to be lockless.
*/
set_mm_exe_file(bprm->mm, bprm->file);
retval = set_mm_exe_file(bprm->mm, bprm->file);
if (retval)
goto out;

/* If the binary is not readable then enforce mm->dumpable=0 */
would_dump(bprm, bprm->file);
Expand Down
1 change: 0 additions & 1 deletion fs/proc/task_mmu.c
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,6 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_MAYSHARE)] = "ms",
[ilog2(VM_GROWSDOWN)] = "gd",
[ilog2(VM_PFNMAP)] = "pf",
[ilog2(VM_DENYWRITE)] = "dw",
[ilog2(VM_LOCKED)] = "lo",
[ilog2(VM_IO)] = "io",
[ilog2(VM_SEQ_READ)] = "sr",
Expand Down
19 changes: 12 additions & 7 deletions include/linux/fs.h
Original file line number Diff line number Diff line change
Expand Up @@ -3023,15 +3023,20 @@ static inline void file_end_write(struct file *file)
}

/*
* This is used for regular files where some users -- especially the
* currently executed binary in a process, previously handled via
* VM_DENYWRITE -- cannot handle concurrent write (and maybe mmap
* read-write shared) accesses.
*
* get_write_access() gets write permission for a file.
* put_write_access() releases this write permission.
* This is used for regular files.
* We cannot support write (and maybe mmap read-write shared) accesses and
* MAP_DENYWRITE mmappings simultaneously. The i_writecount field of an inode
* can have the following values:
* 0: no writers, no VM_DENYWRITE mappings
* < 0: (-i_writecount) vm_area_structs with VM_DENYWRITE set exist
* > 0: (i_writecount) users are writing to the file.
* deny_write_access() denies write access to a file.
* allow_write_access() re-enables write access to a file.
*
* The i_writecount field of an inode can have the following values:
* 0: no write access, no denied write access
* < 0: (-i_writecount) users that denied write access to the file.
* > 0: (i_writecount) users that have write access to the file.
*
* Normally we operate on that counter with atomic_{inc,dec} and it's safe
* except for the cases where we don't hold i_writecount yet. Then we need to
Expand Down
4 changes: 2 additions & 2 deletions include/linux/mm.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ extern unsigned int kobjsize(const void *objp);
#define VM_GROWSDOWN 0x00000100 /* general info on the segment */
#define VM_UFFD_MISSING 0x00000200 /* missing pages tracking */
#define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
#define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
#define VM_UFFD_WP 0x00001000 /* wrprotect pages tracking */

#define VM_LOCKED 0x00002000
Expand Down Expand Up @@ -2573,7 +2572,8 @@ static inline int check_data_rlimit(unsigned long rlim,
extern int mm_take_all_locks(struct mm_struct *mm);
extern void mm_drop_all_locks(struct mm_struct *mm);

extern void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file);
extern int set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file);
extern int replace_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file);
extern struct file *get_mm_exe_file(struct mm_struct *mm);
extern struct file *get_task_exe_file(struct task_struct *task);

Expand Down
4 changes: 2 additions & 2 deletions include/linux/mman.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
* The historical set of flags that all mmap implementations implicitly
* support when a ->mmap_validate() op is not provided in file_operations.
*
* MAP_EXECUTABLE is completely ignored throughout the kernel.
* MAP_EXECUTABLE and MAP_DENYWRITE are completely ignored throughout the
* kernel.
*/
#define LEGACY_MAP_MASK (MAP_SHARED \
| MAP_PRIVATE \
Expand Down Expand Up @@ -153,7 +154,6 @@ static inline unsigned long
calc_vm_flag_bits(unsigned long flags)
{
return _calc_vm_trans(flags, MAP_GROWSDOWN, VM_GROWSDOWN ) |
_calc_vm_trans(flags, MAP_DENYWRITE, VM_DENYWRITE ) |
_calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED ) |
_calc_vm_trans(flags, MAP_SYNC, VM_SYNC ) |
arch_calc_vm_flag_bits(flags);
Expand Down
1 change: 0 additions & 1 deletion include/trace/events/mmflags.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ IF_HAVE_PG_SKIP_KASAN_POISON(PG_skip_kasan_poison, "skip_kasan_poison")
{VM_UFFD_MISSING, "uffd_missing" }, \
IF_HAVE_UFFD_MINOR(VM_UFFD_MINOR, "uffd_minor" ) \
{VM_PFNMAP, "pfnmap" }, \
{VM_DENYWRITE, "denywrite" }, \
{VM_UFFD_WP, "uffd_wp" }, \
{VM_LOCKED, "locked" }, \
{VM_IO, "io" }, \
Expand Down
2 changes: 0 additions & 2 deletions kernel/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -8320,8 +8320,6 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
else
flags = MAP_PRIVATE;

if (vma->vm_flags & VM_DENYWRITE)
flags |= MAP_DENYWRITE;
if (vma->vm_flags & VM_LOCKED)
flags |= MAP_LOCKED;
if (is_vm_hugetlb_page(vma))
Expand Down
95 changes: 84 additions & 11 deletions kernel/fork.c
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,20 @@ void free_task(struct task_struct *tsk)
}
EXPORT_SYMBOL(free_task);

static void dup_mm_exe_file(struct mm_struct *mm, struct mm_struct *oldmm)
{
struct file *exe_file;

exe_file = get_mm_exe_file(oldmm);
RCU_INIT_POINTER(mm->exe_file, exe_file);
/*
* We depend on the oldmm having properly denied write access to the
* exe_file already.
*/
if (exe_file && deny_write_access(exe_file))
pr_warn_once("deny_write_access() failed in %s\n", __func__);
}

#ifdef CONFIG_MMU
static __latent_entropy int dup_mmap(struct mm_struct *mm,
struct mm_struct *oldmm)
Expand All @@ -494,7 +508,7 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
mmap_write_lock_nested(mm, SINGLE_DEPTH_NESTING);

/* No ordering required: file already has been exposed. */
RCU_INIT_POINTER(mm->exe_file, get_mm_exe_file(oldmm));
dup_mm_exe_file(mm, oldmm);

mm->total_vm = oldmm->total_vm;
mm->data_vm = oldmm->data_vm;
Expand Down Expand Up @@ -557,12 +571,9 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm,
tmp->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
file = tmp->vm_file;
if (file) {
struct inode *inode = file_inode(file);
struct address_space *mapping = file->f_mapping;

get_file(file);
if (tmp->vm_flags & VM_DENYWRITE)
put_write_access(inode);
i_mmap_lock_write(mapping);
if (tmp->vm_flags & VM_SHARED)
mapping_allow_writable(mapping);
Expand Down Expand Up @@ -640,7 +651,7 @@ static inline void mm_free_pgd(struct mm_struct *mm)
static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
{
mmap_write_lock(oldmm);
RCU_INIT_POINTER(mm->exe_file, get_mm_exe_file(oldmm));
dup_mm_exe_file(mm, oldmm);
mmap_write_unlock(oldmm);
return 0;
}
Expand Down Expand Up @@ -1150,11 +1161,11 @@ void mmput_async(struct mm_struct *mm)
*
* Main users are mmput() and sys_execve(). Callers prevent concurrent
* invocations: in mmput() nobody alive left, in execve task is single
* threaded. sys_prctl(PR_SET_MM_MAP/EXE_FILE) also needs to set the
* mm->exe_file, but does so without using set_mm_exe_file() in order
* to avoid the need for any locks.
* threaded.
*
* Can only fail if new_exe_file != NULL.
*/
void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
int set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
{
struct file *old_exe_file;

Expand All @@ -1165,11 +1176,73 @@ void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
*/
old_exe_file = rcu_dereference_raw(mm->exe_file);

if (new_exe_file)
if (new_exe_file) {
/*
* We expect the caller (i.e., sys_execve) to already denied
* write access, so this is unlikely to fail.
*/
if (unlikely(deny_write_access(new_exe_file)))
return -EACCES;
get_file(new_exe_file);
}
rcu_assign_pointer(mm->exe_file, new_exe_file);
if (old_exe_file)
if (old_exe_file) {
allow_write_access(old_exe_file);
fput(old_exe_file);
}
return 0;
}

/**
* replace_mm_exe_file - replace a reference to the mm's executable file
*
* This changes mm's executable file (shown as symlink /proc/[pid]/exe),
* dealing with concurrent invocation and without grabbing the mmap lock in
* write mode.
*
* Main user is sys_prctl(PR_SET_MM_MAP/EXE_FILE).
*/
int replace_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
{
struct vm_area_struct *vma;
struct file *old_exe_file;
int ret = 0;

/* Forbid mm->exe_file change if old file still mapped. */
old_exe_file = get_mm_exe_file(mm);
if (old_exe_file) {
mmap_read_lock(mm);
for (vma = mm->mmap; vma && !ret; vma = vma->vm_next) {
if (!vma->vm_file)
continue;
if (path_equal(&vma->vm_file->f_path,
&old_exe_file->f_path))
ret = -EBUSY;
}
mmap_read_unlock(mm);
fput(old_exe_file);
if (ret)
return ret;
}

/* set the new file, lockless */
ret = deny_write_access(new_exe_file);
if (ret)
return -EACCES;
get_file(new_exe_file);

old_exe_file = xchg(&mm->exe_file, new_exe_file);
if (old_exe_file) {
/*
* Don't race with dup_mmap() getting the file and disallowing
* write access while someone might open the file writable.
*/
mmap_read_lock(mm);
allow_write_access(old_exe_file);
fput(old_exe_file);
mmap_read_unlock(mm);
}
return 0;
}

/**
Expand Down
33 changes: 1 addition & 32 deletions kernel/sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,6 @@ SYSCALL_DEFINE1(umask, int, mask)
static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
{
struct fd exe;
struct file *old_exe, *exe_file;
struct inode *inode;
int err;

Expand All @@ -1870,40 +1869,10 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
if (err)
goto exit;

/*
* Forbid mm->exe_file change if old file still mapped.
*/
exe_file = get_mm_exe_file(mm);
err = -EBUSY;
if (exe_file) {
struct vm_area_struct *vma;

mmap_read_lock(mm);
for (vma = mm->mmap; vma; vma = vma->vm_next) {
if (!vma->vm_file)
continue;
if (path_equal(&vma->vm_file->f_path,
&exe_file->f_path))
goto exit_err;
}

mmap_read_unlock(mm);
fput(exe_file);
}

err = 0;
/* set the new file, lockless */
get_file(exe.file);
old_exe = xchg(&mm->exe_file, exe.file);
if (old_exe)
fput(old_exe);
err = replace_mm_exe_file(mm, exe.file);
exit:
fdput(exe);
return err;
exit_err:
mmap_read_unlock(mm);
fput(exe_file);
goto exit;
}

/*
Expand Down
5 changes: 2 additions & 3 deletions lib/test_printf.c
Original file line number Diff line number Diff line change
Expand Up @@ -675,9 +675,8 @@ flags(void)
"uptodate|dirty|lru|active|swapbacked",
cmp_buffer);

flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC
| VM_DENYWRITE;
test("read|exec|mayread|maywrite|mayexec|denywrite", "%pGv", &flags);
flags = VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
test("read|exec|mayread|maywrite|mayexec", "%pGv", &flags);

gfp = GFP_TRANSHUGE;
test("GFP_TRANSHUGE", "%pGg", &gfp);
Expand Down
Loading

0 comments on commit 49624ef

Please sign in to comment.