* Eric W. Biederman <ebiederm@xxxxxxxxxxxx> [210430 15:57]:
> Liam Howlett <liam.howlett@xxxxxxxxxx> writes:
>
> > This is way out of scope for what I'm doing. I'm trying to fix a call
> > to the wrong mm API. I was trying to clean up any obvious errors in
> > calling functions which were exposed by fixing that error. If you want
> > this fixed differently, then please go ahead and tackle the problems you
> > see.
>
> I was asked by the arm maintainers to describe what the code should be
> doing here. I hope I have done that.
>
> What is very interesting is that the code in __do_page_fault does not
> use find_vma_intersection it uses find_vma. Which suggests that
> find_vma_intersection may not be the proper mm api.
>
> The logic is:
>
> From __do_page_fault:
> struct vm_area_struct *vma = find_vma(mm, addr);
>
> if (unlikely(!vma))
> return VM_FAULT_BADMAP;
>
> /*
> * Ok, we have a good vm_area for this memory access, so we can handle
> * it.
> */
> if (unlikely(vma->vm_start > addr)) {
> if (!(vma->vm_flags & VM_GROWSDOWN))
> return VM_FAULT_BADMAP;
> if (expand_stack(vma, addr))
> return VM_FAULT_BADMAP;
> }
>
> /*
> * Check that the permissions on the VMA allow for the fault which
> * occurred.
> */
> if (!(vma->vm_flags & vm_flags))
> return VM_FAULT_BADACCESS;
>
> From do_page_fault:
>
> arm64_force_sig_fault(SIGSEGV,
> fault == VM_FAULT_BADACCESS ? SEGV_ACCERR : SEGV_MAPERR,
> far, inf->name);
>
>
> Hmm. If the expand_stack step is skipped. Does is the logic equivalent
> to find_vma_intersection?
>
> static inline struct vm_area_struct *find_vma_intersection(
> struct mm_struct * mm,
> unsigned long start_addr,
> unsigned long end_addr)
> {
> struct vm_area_struct * vma = find_vma(mm,start_addr);
>
> if (vma && end_addr <= vma->vm_start)
> vma = NULL;
> return vma;
> }
>
> Yes. It does look that way. VM_FAULT_BADMAP is returned when a vma
> covering the specified address is not found. And VM_FAULT_BADACCESS is
> returned when there is a vma and there is a permission problem.
>
> There are also two SIGBUS cases that arm64_notify_segfault does not
> handle.
>
> So it appears changing arm64_notify_segfault to use
> find_vma_intersection instead of find_vma would be a correct but
> incomplete fix.
The reason VM_GROWSDOWN is checked here is to see if the stack should be
attempted to be expanded, which happens above. If VM_GROWSDOWN is true,
then wouldn't the do_page_fault() and __do_page_fault() calls already be
done and be handling this case?
>
> I don't see a point in changing sigerturn or rt_sigreturn.
Both functions call arm64_notify_segfault() on !access_ok() which I've
increased the potential for returning SEGV_MAPERR. It is also not
necessary to walk the VMAs when !access_ok(), so it seemed a decent
thing to do.
Thanks,
Liam
