Skip to content

Commit

Permalink
mm: always expand the stack with the mmap write lock held
Browse files Browse the repository at this point in the history
commit 8d7071a upstream.

This finishes the job of always holding the mmap write lock when
extending the user stack vma, and removes the 'write_locked' argument
from the vm helper functions again.

For some cases, we just avoid expanding the stack at all: drivers and
page pinning really shouldn't be extending any stacks.  Let's see if any
strange users really wanted that.

It's worth noting that architectures that weren't converted to the new
lock_mm_and_find_vma() helper function are left using the legacy
"expand_stack()" function, but it has been changed to drop the mmap_lock
and take it for writing while expanding the vma.  This makes it fairly
straightforward to convert the remaining architectures.

As a result of dropping and re-taking the lock, the calling conventions
for this function have also changed, since the old vma may no longer be
valid.  So it will now return the new vma if successful, and NULL - and
the lock dropped - if the area could not be extended.

Tested-by: Vegard Nossum <vegard.nossum@oracle.com>
Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> # ia64
Tested-by: Frank Scheiner <frank.scheiner@web.de> # ia64
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
torvalds authored and gregkh committed Jul 1, 2023
1 parent af099fa commit fb32951
Show file tree
Hide file tree
Showing 17 changed files with 169 additions and 116 deletions.
36 changes: 6 additions & 30 deletions arch/ia64/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,12 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re
* register backing store that needs to expand upwards, in
* this case vma will be null, but prev_vma will ne non-null
*/
if (( !vma && prev_vma ) || (address < vma->vm_start) )
goto check_expansion;
if (( !vma && prev_vma ) || (address < vma->vm_start) ) {
vma = expand_stack(mm, address);
if (!vma)
goto bad_area_nosemaphore;
}

good_area:
code = SEGV_ACCERR;

/* OK, we've got a good vm_area for this memory area. Check the access permissions: */
Expand Down Expand Up @@ -177,35 +179,9 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re
mmap_read_unlock(mm);
return;

check_expansion:
if (!(prev_vma && (prev_vma->vm_flags & VM_GROWSUP) && (address == prev_vma->vm_end))) {
if (!vma)
goto bad_area;
if (!(vma->vm_flags & VM_GROWSDOWN))
goto bad_area;
if (REGION_NUMBER(address) != REGION_NUMBER(vma->vm_start)
|| REGION_OFFSET(address) >= RGN_MAP_LIMIT)
goto bad_area;
if (expand_stack(vma, address))
goto bad_area;
} else {
vma = prev_vma;
if (REGION_NUMBER(address) != REGION_NUMBER(vma->vm_start)
|| REGION_OFFSET(address) >= RGN_MAP_LIMIT)
goto bad_area;
/*
* Since the register backing store is accessed sequentially,
* we disallow growing it by more than a page at a time.
*/
if (address > vma->vm_end + PAGE_SIZE - sizeof(long))
goto bad_area;
if (expand_upwards(vma, address))
goto bad_area;
}
goto good_area;

bad_area:
mmap_read_unlock(mm);
bad_area_nosemaphore:
if ((isr & IA64_ISR_SP)
|| ((isr & IA64_ISR_NA) && (isr & IA64_ISR_CODE_MASK) == IA64_ISR_CODE_LFETCH))
{
Expand Down
9 changes: 6 additions & 3 deletions arch/m68k/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
if (address + 256 < rdusp())
goto map_err;
}
if (expand_stack(vma, address))
goto map_err;
vma = expand_stack(mm, address);
if (!vma)
goto map_err_nosemaphore;

/*
* Ok, we have a good vm_area for this memory access, so
Expand Down Expand Up @@ -196,10 +197,12 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
goto send_sig;

map_err:
mmap_read_unlock(mm);
map_err_nosemaphore:
current->thread.signo = SIGSEGV;
current->thread.code = SEGV_MAPERR;
current->thread.faddr = address;
goto send_sig;
return send_fault_sig(regs);

acc_err:
current->thread.signo = SIGSEGV;
Expand Down
5 changes: 3 additions & 2 deletions arch/microblaze/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,9 @@ void do_page_fault(struct pt_regs *regs, unsigned long address,
&& (kernel_mode(regs) || !store_updates_sp(regs)))
goto bad_area;
}
if (expand_stack(vma, address))
goto bad_area;
vma = expand_stack(mm, address);
if (!vma)
goto bad_area_nosemaphore;

good_area:
code = SEGV_ACCERR;
Expand Down
5 changes: 3 additions & 2 deletions arch/openrisc/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,9 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address,
if (address + PAGE_SIZE < regs->sp)
goto bad_area;
}
if (expand_stack(vma, address))
goto bad_area;
vma = expand_stack(mm, address);
if (!vma)
goto bad_area_nosemaphore;

/*
* Ok, we have a good vm_area for this memory access, so
Expand Down
23 changes: 11 additions & 12 deletions arch/parisc/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,19 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,
retry:
mmap_read_lock(mm);
vma = find_vma_prev(mm, address, &prev_vma);
if (!vma || address < vma->vm_start)
goto check_expansion;
if (!vma || address < vma->vm_start) {
if (!prev || !(prev->vm_flags & VM_GROWSUP))
goto bad_area;
vma = expand_stack(mm, address);
if (!vma)
goto bad_area_nosemaphore;
}

/*
* Ok, we have a good vm_area for this memory access. We still need to
* check the access permissions.
*/

good_area:

if ((vma->vm_flags & acc_type) != acc_type)
goto bad_area;

Expand Down Expand Up @@ -347,17 +351,13 @@ void do_page_fault(struct pt_regs *regs, unsigned long code,
mmap_read_unlock(mm);
return;

check_expansion:
vma = prev_vma;
if (vma && (expand_stack(vma, address) == 0))
goto good_area;

/*
* Something tried to access memory that isn't in our memory map..
*/
bad_area:
mmap_read_unlock(mm);

bad_area_nosemaphore:
if (user_mode(regs)) {
int signo, si_code;

Expand Down Expand Up @@ -449,7 +449,7 @@ handle_nadtlb_fault(struct pt_regs *regs)
{
unsigned long insn = regs->iir;
int breg, treg, xreg, val = 0;
struct vm_area_struct *vma, *prev_vma;
struct vm_area_struct *vma;
struct task_struct *tsk;
struct mm_struct *mm;
unsigned long address;
Expand Down Expand Up @@ -485,7 +485,7 @@ handle_nadtlb_fault(struct pt_regs *regs)
/* Search for VMA */
address = regs->ior;
mmap_read_lock(mm);
vma = find_vma_prev(mm, address, &prev_vma);
vma = vma_lookup(mm, address);
mmap_read_unlock(mm);

/*
Expand All @@ -494,7 +494,6 @@ handle_nadtlb_fault(struct pt_regs *regs)
*/
acc_type = (insn & 0x40) ? VM_WRITE : VM_READ;
if (vma
&& address >= vma->vm_start
&& (vma->vm_flags & acc_type) == acc_type)
val = 1;
}
Expand Down
5 changes: 3 additions & 2 deletions arch/s390/mm/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,9 @@ static inline vm_fault_t do_exception(struct pt_regs *regs, int access)
if (unlikely(vma->vm_start > address)) {
if (!(vma->vm_flags & VM_GROWSDOWN))
goto out_up;
if (expand_stack(vma, address))
goto out_up;
vma = expand_stack(mm, address);
if (!vma)
goto out;
}

/*
Expand Down
8 changes: 5 additions & 3 deletions arch/sparc/mm/fault_64.c
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,9 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs)
goto bad_area;
}
}
if (expand_stack(vma, address))
goto bad_area;
vma = expand_stack(mm, address);
if (!vma)
goto bad_area_nosemaphore;
/*
* Ok, we have a good vm_area for this memory access, so
* we can handle it..
Expand Down Expand Up @@ -487,8 +488,9 @@ asmlinkage void __kprobes do_sparc64_fault(struct pt_regs *regs)
* Fix it, but check if it's kernel or user first..
*/
bad_area:
insn = get_fault_insn(regs, insn);
mmap_read_unlock(mm);
bad_area_nosemaphore:
insn = get_fault_insn(regs, insn);

handle_kernel_fault:
do_kernel_fault(regs, si_code, fault_code, insn, address);
Expand Down
11 changes: 6 additions & 5 deletions arch/um/kernel/trap.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,15 @@ int handle_page_fault(unsigned long address, unsigned long ip,
vma = find_vma(mm, address);
if (!vma)
goto out;
else if (vma->vm_start <= address)
if (vma->vm_start <= address)
goto good_area;
else if (!(vma->vm_flags & VM_GROWSDOWN))
if (!(vma->vm_flags & VM_GROWSDOWN))
goto out;
else if (is_user && !ARCH_IS_STACKGROW(address))
goto out;
else if (expand_stack(vma, address))
if (is_user && !ARCH_IS_STACKGROW(address))
goto out;
vma = expand_stack(mm, address);
if (!vma)
goto out_nosemaphore;

good_area:
*code_out = SEGV_ACCERR;
Expand Down
4 changes: 2 additions & 2 deletions drivers/iommu/amd/iommu_v2.c
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,8 @@ static void do_fault(struct work_struct *work)
flags |= FAULT_FLAG_REMOTE;

mmap_read_lock(mm);
vma = find_extend_vma(mm, address);
if (!vma || address < vma->vm_start)
vma = vma_lookup(mm, address);
if (!vma)
/* failed to get a vma in the right range */
goto out;

Expand Down
2 changes: 1 addition & 1 deletion drivers/iommu/iommu-sva.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)

mmap_read_lock(mm);

vma = find_extend_vma(mm, prm->addr);
vma = vma_lookup(mm, prm->addr);
if (!vma)
/* Unmapped area */
goto out_put_mm;
Expand Down
2 changes: 1 addition & 1 deletion fs/binfmt_elf.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
*/
if (mmap_write_lock_killable(mm))
return -EINTR;
vma = find_extend_vma_locked(mm, bprm->p, true);
vma = find_extend_vma_locked(mm, bprm->p);
mmap_write_unlock(mm);
if (!vma)
return -EFAULT;
Expand Down
4 changes: 2 additions & 2 deletions fs/exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
*/
if (write && pos < vma->vm_start) {
mmap_write_lock(mm);
ret = expand_downwards(vma, pos, true);
ret = expand_downwards(vma, pos);
if (unlikely(ret < 0)) {
mmap_write_unlock(mm);
return NULL;
Expand Down Expand Up @@ -859,7 +859,7 @@ int setup_arg_pages(struct linux_binprm *bprm,
stack_base = vma->vm_end - stack_expand;
#endif
current->mm->start_stack = bprm->p;
ret = expand_stack_locked(vma, stack_base, true);
ret = expand_stack_locked(vma, stack_base);
if (ret)
ret = -EFAULT;

Expand Down
16 changes: 4 additions & 12 deletions include/linux/mm.h
Original file line number Diff line number Diff line change
Expand Up @@ -3192,18 +3192,11 @@ extern vm_fault_t filemap_page_mkwrite(struct vm_fault *vmf);

extern unsigned long stack_guard_gap;
/* Generic expand stack which grows the stack according to GROWS{UP,DOWN} */
int expand_stack_locked(struct vm_area_struct *vma, unsigned long address,
bool write_locked);
#define expand_stack(vma,addr) expand_stack_locked(vma,addr,false)
int expand_stack_locked(struct vm_area_struct *vma, unsigned long address);
struct vm_area_struct *expand_stack(struct mm_struct * mm, unsigned long addr);

/* CONFIG_STACK_GROWSUP still needs to grow downwards at some places */
int expand_downwards(struct vm_area_struct *vma, unsigned long address,
bool write_locked);
#if VM_GROWSUP
extern int expand_upwards(struct vm_area_struct *vma, unsigned long address);
#else
#define expand_upwards(vma, address) (0)
#endif
int expand_downwards(struct vm_area_struct *vma, unsigned long address);

/* Look up the first VMA which satisfies addr < vm_end, NULL if none. */
extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long addr);
Expand Down Expand Up @@ -3298,9 +3291,8 @@ unsigned long change_prot_numa(struct vm_area_struct *vma,
unsigned long start, unsigned long end);
#endif

struct vm_area_struct *find_extend_vma(struct mm_struct *, unsigned long addr);
struct vm_area_struct *find_extend_vma_locked(struct mm_struct *,
unsigned long addr, bool write_locked);
unsigned long addr);
int remap_pfn_range(struct vm_area_struct *, unsigned long addr,
unsigned long pfn, unsigned long size, pgprot_t);
int remap_pfn_range_notrack(struct vm_area_struct *vma, unsigned long addr,
Expand Down
6 changes: 3 additions & 3 deletions mm/gup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1096,7 +1096,7 @@ static long __get_user_pages(struct mm_struct *mm,

/* first iteration or cross vma bound */
if (!vma || start >= vma->vm_end) {
vma = find_extend_vma(mm, start);
vma = vma_lookup(mm, start);
if (!vma && in_gate_area(mm, start)) {
ret = get_gate_page(mm, start & PAGE_MASK,
gup_flags, &vma,
Expand Down Expand Up @@ -1265,8 +1265,8 @@ int fixup_user_fault(struct mm_struct *mm,
fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;

retry:
vma = find_extend_vma(mm, address);
if (!vma || address < vma->vm_start)
vma = vma_lookup(mm, address);
if (!vma)
return -EFAULT;

if (!vma_permits_fault(vma, fault_flags))
Expand Down
10 changes: 9 additions & 1 deletion mm/memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -5368,7 +5368,7 @@ struct vm_area_struct *lock_mm_and_find_vma(struct mm_struct *mm,
goto fail;
}

if (expand_stack_locked(vma, addr, true))
if (expand_stack_locked(vma, addr))
goto fail;

success:
Expand Down Expand Up @@ -5713,6 +5713,14 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
if (mmap_read_lock_killable(mm))
return 0;

/* We might need to expand the stack to access it */
vma = vma_lookup(mm, addr);
if (!vma) {
vma = expand_stack(mm, addr);
if (!vma)
return 0;
}

/* ignore errors, just check how much was successfully transferred */
while (len) {
int bytes, ret, offset;
Expand Down

0 comments on commit fb32951

Please sign in to comment.