diff for duplicates of <20160829124233.GA40092@black.fi.intel.com> diff --git a/a/1.txt b/N1/1.txt index 2640c75..dea98c6 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -22,3 +22,112 @@ On Sun, Aug 28, 2016 at 12:42:21PM +0200, Dmitry Vyukov wrote: Okay, I think the patch below should do the trick. Build tested only. Andrea, Ebru, could you re-check if it's reasonable. + +>From bc6c3589a3da75fabd6d440cce3c8ea23b69c2e5 Mon Sep 17 00:00:00 2001 +From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> +Date: Mon, 29 Aug 2016 15:32:50 +0300 +Subject: [PATCH] khugepaged: fix use-after-free in collapse_huge_page() + +hugepage_vma_revalidate() tries to re-check if we still should try to +collapse small pages into huge one after the re-acquiring mmap_sem. + +The problem Dmitry Vyukov reported[1] is that the vma found by +hugepage_vma_revalidate() can be suitable for huge pages, but not the +same vma we had before dropping mmap_sem. And dereferencing original vma +can lead to fun results.. + +Let's use vma hugepage_vma_revalidate() found instead of assuming it's +the same as what we had before the lock was dropped. + +[1] http://lkml.kernel.org/r/CACT4Y+Z3gigBvhca9kRJFcjX0G70V_nRhbwKBU+yGoESBDKi9Q@mail.gmail.com + +Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> +Reported-by: Dmitry Vyukov <dvyukov@google.com> +--- + mm/khugepaged.c | 17 +++++++++-------- + 1 file changed, 9 insertions(+), 8 deletions(-) + +diff --git a/mm/khugepaged.c b/mm/khugepaged.c +index 79c52d0061af..3c8253f160ca 100644 +--- a/mm/khugepaged.c ++++ b/mm/khugepaged.c +@@ -838,7 +838,8 @@ static bool hugepage_vma_check(struct vm_area_struct *vma) + * value (scan code). + */ + +-static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address) ++static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address, ++ struct vm_area_struct **vmap) + { + struct vm_area_struct *vma; + unsigned long hstart, hend; +@@ -846,7 +847,7 @@ static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address) + if (unlikely(khugepaged_test_exit(mm))) + return SCAN_ANY_PROCESS; + +- vma = find_vma(mm, address); ++ *vmap = vma = find_vma(mm, address); + if (!vma) + return SCAN_VMA_NULL; + +@@ -898,13 +899,13 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm, + /* do_swap_page returns VM_FAULT_RETRY with released mmap_sem */ + if (ret & VM_FAULT_RETRY) { + down_read(&mm->mmap_sem); +- if (hugepage_vma_revalidate(mm, address)) { ++ if (hugepage_vma_revalidate(mm, address, &vma)) { + /* vma is no longer available, don't continue to swapin */ + trace_mm_collapse_huge_page_swapin(mm, swapped_in, referenced, 0); + return false; + } + /* check if the pmd is still valid */ +- if (mm_find_pmd(mm, address) != pmd) ++ if (mm_find_pmd(mm, address) != pmd || vma != fe.vma) + return false; + } + if (ret & VM_FAULT_ERROR) { +@@ -923,7 +924,6 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm, + static void collapse_huge_page(struct mm_struct *mm, + unsigned long address, + struct page **hpage, +- struct vm_area_struct *vma, + int node, int referenced) + { + pmd_t *pmd, _pmd; +@@ -933,6 +933,7 @@ static void collapse_huge_page(struct mm_struct *mm, + spinlock_t *pmd_ptl, *pte_ptl; + int isolated = 0, result = 0; + struct mem_cgroup *memcg; ++ struct vm_area_struct *vma; + unsigned long mmun_start; /* For mmu_notifiers */ + unsigned long mmun_end; /* For mmu_notifiers */ + gfp_t gfp; +@@ -961,7 +962,7 @@ static void collapse_huge_page(struct mm_struct *mm, + } + + down_read(&mm->mmap_sem); +- result = hugepage_vma_revalidate(mm, address); ++ result = hugepage_vma_revalidate(mm, address, &vma); + if (result) { + mem_cgroup_cancel_charge(new_page, memcg, true); + up_read(&mm->mmap_sem); +@@ -994,7 +995,7 @@ static void collapse_huge_page(struct mm_struct *mm, + * handled by the anon_vma lock + PG_lock. + */ + down_write(&mm->mmap_sem); +- result = hugepage_vma_revalidate(mm, address); ++ result = hugepage_vma_revalidate(mm, address, &vma); + if (result) + goto out; + /* check if the pmd is still valid */ +@@ -1202,7 +1203,7 @@ out_unmap: + if (ret) { + node = khugepaged_find_target_node(); + /* collapse_huge_page will return with the mmap_sem released */ +- collapse_huge_page(mm, address, hpage, vma, node, referenced); ++ collapse_huge_page(mm, address, hpage, node, referenced); + } + out: + trace_mm_khugepaged_scan_pmd(mm, page, writable, referenced, +-- + Kirill A. Shutemov diff --git a/a/content_digest b/N1/content_digest index a5ae11e..d6fc1c8 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -47,6 +47,115 @@ "\n" "Okay, I think the patch below should do the trick. Build tested only.\n" "\n" - Andrea, Ebru, could you re-check if it's reasonable. + "Andrea, Ebru, could you re-check if it's reasonable.\n" + "\n" + ">From bc6c3589a3da75fabd6d440cce3c8ea23b69c2e5 Mon Sep 17 00:00:00 2001\n" + "From: \"Kirill A. Shutemov\" <kirill.shutemov@linux.intel.com>\n" + "Date: Mon, 29 Aug 2016 15:32:50 +0300\n" + "Subject: [PATCH] khugepaged: fix use-after-free in collapse_huge_page()\n" + "\n" + "hugepage_vma_revalidate() tries to re-check if we still should try to\n" + "collapse small pages into huge one after the re-acquiring mmap_sem.\n" + "\n" + "The problem Dmitry Vyukov reported[1] is that the vma found by\n" + "hugepage_vma_revalidate() can be suitable for huge pages, but not the\n" + "same vma we had before dropping mmap_sem. And dereferencing original vma\n" + "can lead to fun results..\n" + "\n" + "Let's use vma hugepage_vma_revalidate() found instead of assuming it's\n" + "the same as what we had before the lock was dropped.\n" + "\n" + "[1] http://lkml.kernel.org/r/CACT4Y+Z3gigBvhca9kRJFcjX0G70V_nRhbwKBU+yGoESBDKi9Q@mail.gmail.com\n" + "\n" + "Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>\n" + "Reported-by: Dmitry Vyukov <dvyukov@google.com>\n" + "---\n" + " mm/khugepaged.c | 17 +++++++++--------\n" + " 1 file changed, 9 insertions(+), 8 deletions(-)\n" + "\n" + "diff --git a/mm/khugepaged.c b/mm/khugepaged.c\n" + "index 79c52d0061af..3c8253f160ca 100644\n" + "--- a/mm/khugepaged.c\n" + "+++ b/mm/khugepaged.c\n" + "@@ -838,7 +838,8 @@ static bool hugepage_vma_check(struct vm_area_struct *vma)\n" + " * value (scan code).\n" + " */\n" + " \n" + "-static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address)\n" + "+static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address,\n" + "+\t\tstruct vm_area_struct **vmap)\n" + " {\n" + " \tstruct vm_area_struct *vma;\n" + " \tunsigned long hstart, hend;\n" + "@@ -846,7 +847,7 @@ static int hugepage_vma_revalidate(struct mm_struct *mm, unsigned long address)\n" + " \tif (unlikely(khugepaged_test_exit(mm)))\n" + " \t\treturn SCAN_ANY_PROCESS;\n" + " \n" + "-\tvma = find_vma(mm, address);\n" + "+\t*vmap = vma = find_vma(mm, address);\n" + " \tif (!vma)\n" + " \t\treturn SCAN_VMA_NULL;\n" + " \n" + "@@ -898,13 +899,13 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm,\n" + " \t\t/* do_swap_page returns VM_FAULT_RETRY with released mmap_sem */\n" + " \t\tif (ret & VM_FAULT_RETRY) {\n" + " \t\t\tdown_read(&mm->mmap_sem);\n" + "-\t\t\tif (hugepage_vma_revalidate(mm, address)) {\n" + "+\t\t\tif (hugepage_vma_revalidate(mm, address, &vma)) {\n" + " \t\t\t\t/* vma is no longer available, don't continue to swapin */\n" + " \t\t\t\ttrace_mm_collapse_huge_page_swapin(mm, swapped_in, referenced, 0);\n" + " \t\t\t\treturn false;\n" + " \t\t\t}\n" + " \t\t\t/* check if the pmd is still valid */\n" + "-\t\t\tif (mm_find_pmd(mm, address) != pmd)\n" + "+\t\t\tif (mm_find_pmd(mm, address) != pmd || vma != fe.vma)\n" + " \t\t\t\treturn false;\n" + " \t\t}\n" + " \t\tif (ret & VM_FAULT_ERROR) {\n" + "@@ -923,7 +924,6 @@ static bool __collapse_huge_page_swapin(struct mm_struct *mm,\n" + " static void collapse_huge_page(struct mm_struct *mm,\n" + " \t\t\t\t unsigned long address,\n" + " \t\t\t\t struct page **hpage,\n" + "-\t\t\t\t struct vm_area_struct *vma,\n" + " \t\t\t\t int node, int referenced)\n" + " {\n" + " \tpmd_t *pmd, _pmd;\n" + "@@ -933,6 +933,7 @@ static void collapse_huge_page(struct mm_struct *mm,\n" + " \tspinlock_t *pmd_ptl, *pte_ptl;\n" + " \tint isolated = 0, result = 0;\n" + " \tstruct mem_cgroup *memcg;\n" + "+\tstruct vm_area_struct *vma;\n" + " \tunsigned long mmun_start;\t/* For mmu_notifiers */\n" + " \tunsigned long mmun_end;\t\t/* For mmu_notifiers */\n" + " \tgfp_t gfp;\n" + "@@ -961,7 +962,7 @@ static void collapse_huge_page(struct mm_struct *mm,\n" + " \t}\n" + " \n" + " \tdown_read(&mm->mmap_sem);\n" + "-\tresult = hugepage_vma_revalidate(mm, address);\n" + "+\tresult = hugepage_vma_revalidate(mm, address, &vma);\n" + " \tif (result) {\n" + " \t\tmem_cgroup_cancel_charge(new_page, memcg, true);\n" + " \t\tup_read(&mm->mmap_sem);\n" + "@@ -994,7 +995,7 @@ static void collapse_huge_page(struct mm_struct *mm,\n" + " \t * handled by the anon_vma lock + PG_lock.\n" + " \t */\n" + " \tdown_write(&mm->mmap_sem);\n" + "-\tresult = hugepage_vma_revalidate(mm, address);\n" + "+\tresult = hugepage_vma_revalidate(mm, address, &vma);\n" + " \tif (result)\n" + " \t\tgoto out;\n" + " \t/* check if the pmd is still valid */\n" + "@@ -1202,7 +1203,7 @@ out_unmap:\n" + " \tif (ret) {\n" + " \t\tnode = khugepaged_find_target_node();\n" + " \t\t/* collapse_huge_page will return with the mmap_sem released */\n" + "-\t\tcollapse_huge_page(mm, address, hpage, vma, node, referenced);\n" + "+\t\tcollapse_huge_page(mm, address, hpage, node, referenced);\n" + " \t}\n" + " out:\n" + " \ttrace_mm_khugepaged_scan_pmd(mm, page, writable, referenced,\n" + "-- \n" + Kirill A. Shutemov -aaf944da542833ecfffb0d52d89f793cd9443834f1328e643538fe7eeeac2d51 +f32f46b8b1580391646c4636b73dfcdf7483cddfeb492b2bb123a03511a463f1
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.