Messages in this thread Patch in this message | | | From | Peter Xu <> | Subject | [PATCH RFC] mm: arc: fix potential double realease of mmap_sem | Date | Thu, 1 Nov 2018 11:23:54 +0800 |
| |
In do_page_fault() of ARC we have:
... fault = handle_mm_fault(vma, address, flags);
/* If Pagefault was interrupted by SIGKILL, exit page fault "early" */ if (unlikely(fatal_signal_pending(current))) { if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY)) up_read(&mm->mmap_sem); <---------------- [1] if (user_mode(regs)) return; } ... if (likely(!(fault & VM_FAULT_ERROR))) { ... return; }
if (fault & VM_FAULT_OOM) goto out_of_memory; <----------------- [2] else if (fault & VM_FAULT_SIGSEGV) goto bad_area; <----------------- [3] else if (fault & VM_FAULT_SIGBUS) goto do_sigbus; <----------------- [4]
Logically it's possible that we might try to release the mmap_sem twice by having a scenario like:
- task received SIGKILL, - task handled kernel mode page fault, - handle_mm_fault() returned with one of VM_FAULT_ERROR,
Then we'll go into path [1] to release the mmap_sem, however we won't return immediately since user_mode(regs) check will fail (a kernel page fault). Then we might go into either [2]-[4] and either of them will try to release the mmap_sem again.
To fix this, we only release the mmap_sem at [1] when we're sure we'll quit immediately (after we checked with user_mode(regs)).
CC: Vineet Gupta <vgupta@synopsys.com> CC: "Eric W. Biederman" <ebiederm@xmission.com> CC: Peter Xu <peterx@redhat.com> CC: Andrew Morton <akpm@linux-foundation.org> CC: Souptick Joarder <jrdr.linux@gmail.com> CC: Andrea Arcangeli <aarcange@redhat.com> CC: linux-snps-arc@lists.infradead.org CC: linux-kernel@vger.kernel.org Signed-off-by: Peter Xu <peterx@redhat.com> ---
I noticed this only by reading the code. Neither have I verified the issue, nor have I tested the patch since I even don't know how to (I'm totally unfamiliar with the arc architecture). However I'm posting this out first to see whether there's any quick feedback, and in case it's a valid issue that we've ignored. --- arch/arc/mm/fault.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c index c9da6102eb4f..2d28c3dad5c1 100644 --- a/arch/arc/mm/fault.c +++ b/arch/arc/mm/fault.c @@ -142,11 +142,10 @@ void do_page_fault(unsigned long address, struct pt_regs *regs) fault = handle_mm_fault(vma, address, flags); /* If Pagefault was interrupted by SIGKILL, exit page fault "early" */ - if (unlikely(fatal_signal_pending(current))) { - if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY)) + if (unlikely(fatal_signal_pending(current) && user_mode(regs))) { + if (!(fault & VM_FAULT_RETRY)) up_read(&mm->mmap_sem); - if (user_mode(regs)) - return; + return; } perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); -- 2.17.1
| |