Re: Patch "khugepaged: khugepaged_test_exit() check mmget_still_valid()" has been added to the 5.8-stable tree
On Wed, 19 Aug 2020, gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
khugepaged: khugepaged_test_exit() check mmget_still_valid()to the 5.8-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: khugepaged-khugepaged_test_exit-check-mmget_still_valid.patch and it can be found in the queue-5.8 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
Please hold this one back for the moment: we shall want it, but syzbot detected one place where it can lead to a VM_BUG_ON_MM(). The fix to that is currently in Andrew's tree, but not yet in Linus's - when it gets there, I'll send you its git commit id in reply to this mail.
This patch failed to apply to earlier releases: I'll send the fixup for those at that time. (Fixups for another patch to follow later today.)
Thanks, Hugh
From bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 Mon Sep 17 00:00:00 2001 From: Hugh Dickins hughd@google.com Date: Thu, 6 Aug 2020 23:26:25 -0700 Subject: khugepaged: khugepaged_test_exit() check mmget_still_valid()
From: Hugh Dickins hughd@google.com
commit bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 upstream.
Move collapse_huge_page()'s mmget_still_valid() check into khugepaged_test_exit() itself. collapse_huge_page() is used for anon THP only, and earned its mmget_still_valid() check because it inserts a huge pmd entry in place of the page table's pmd entry; whereas collapse_file()'s retract_page_tables() or collapse_pte_mapped_thp() merely clears the page table's pmd entry. But core dumping without mmap lock must have been as open to mistaking a racily cleared pmd entry for a page table at physical page 0, as exit_mmap() was. And we certainly have no interest in mapping as a THP once dumping core.
Fixes: 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") Signed-off-by: Hugh Dickins hughd@google.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Cc: Andrea Arcangeli aarcange@redhat.com Cc: Song Liu songliubraving@fb.com Cc: Mike Kravetz mike.kravetz@oracle.com Cc: Kirill A. Shutemov kirill.shutemov@linux.intel.com Cc: stable@vger.kernel.org [4.8+] Link: http://lkml.kernel.org/r/alpine.LSU.2.11.2008021217020.27773@eggly.anvils Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
mm/khugepaged.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
--- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -431,7 +431,7 @@ static void insert_to_mm_slots_hash(stru static inline int khugepaged_test_exit(struct mm_struct *mm) {
- return atomic_read(&mm->mm_users) == 0;
- return atomic_read(&mm->mm_users) == 0 || !mmget_still_valid(mm);
} static bool hugepage_vma_check(struct vm_area_struct *vma, @@ -1100,9 +1100,6 @@ static void collapse_huge_page(struct mm * handled by the anon_vma lock + PG_lock. */ mmap_write_lock(mm);
- result = SCAN_ANY_PROCESS;
- if (!mmget_still_valid(mm))
goto out;Patches currently in stable-queue which might be from hughd@google.com are
queue-5.8/khugepaged-collapse_pte_mapped_thp-flush-the-right-range.patch queue-5.8/hugetlbfs-remove-call-to-huge_pte_alloc-without-i_mmap_rwsem.patch queue-5.8/khugepaged-khugepaged_test_exit-check-mmget_still_valid.patch queue-5.8/khugepaged-retract_page_tables-remember-to-test-exit.patch queue-5.8/khugepaged-collapse_pte_mapped_thp-protect-the-pmd-lock.patch
On Wed, Aug 19, 2020 at 06:32:26AM -0700, Hugh Dickins wrote:
On Wed, 19 Aug 2020, gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
khugepaged: khugepaged_test_exit() check mmget_still_valid()to the 5.8-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: khugepaged-khugepaged_test_exit-check-mmget_still_valid.patch and it can be found in the queue-5.8 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
Please hold this one back for the moment: we shall want it, but syzbot detected one place where it can lead to a VM_BUG_ON_MM(). The fix to that is currently in Andrew's tree, but not yet in Linus's - when it gets there, I'll send you its git commit id in reply to this mail.
This patch failed to apply to earlier releases: I'll send the fixup for those at that time. (Fixups for another patch to follow later today.)
Now dropped, thanks!
greg k-h
On Wed, 19 Aug 2020, Greg KH wrote:
On Wed, Aug 19, 2020 at 06:32:26AM -0700, Hugh Dickins wrote:
On Wed, 19 Aug 2020, gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
khugepaged: khugepaged_test_exit() check mmget_still_valid()to the 5.8-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: khugepaged-khugepaged_test_exit-check-mmget_still_valid.patch and it can be found in the queue-5.8 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
Please hold this one back for the moment: we shall want it, but syzbot detected one place where it can lead to a VM_BUG_ON_MM(). The fix to that is currently in Andrew's tree, but not yet in Linus's - when it gets there, I'll send you its git commit id in reply to this mail.
This patch failed to apply to earlier releases: I'll send the fixup for those at that time. (Fixups for another patch to follow later today.)
Now dropped, thanks!
f3f99d63a815 khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter() has now reached Linus's tree, so will reach your tree when you next pull.
When that one is ready, please reinstate this commit that we held back: bbe98f9cadff khugepaged: khugepaged_test_exit() check mmget_still_valid()
The mmap_sem->mmap_lock change means I must then send you a backport of bbe98f9cadff for 5.7, 5.4, 4.19, 4.14, 4.9: one backport will do for all of those, and f3f99d63a815 should cherry-pick cleanly into them all.
But you also marked bbe98f9cadff for 4.4: I had not expected that, but I think you're right - for whatever reason (probably inertia, it was tiresome because khugepaged.c got split from huge_memory.c), 4.4 lacks a backport of 59ea6d06cfa9 (though it does have the commit that depended on), and backports of these two will serve just as well to fix what it was required to fix: I'll send them too.
Thanks: I'm sorry that this is all so confusing, kudos to syzbot for catching my error as quickly as it did.
Hugh
On Fri, Aug 21, 2020 at 06:26:15PM -0700, Hugh Dickins wrote:
On Wed, 19 Aug 2020, Greg KH wrote:
On Wed, Aug 19, 2020 at 06:32:26AM -0700, Hugh Dickins wrote:
On Wed, 19 Aug 2020, gregkh@linuxfoundation.org wrote:
This is a note to let you know that I've just added the patch titled
khugepaged: khugepaged_test_exit() check mmget_still_valid()to the 5.8-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: khugepaged-khugepaged_test_exit-check-mmget_still_valid.patch and it can be found in the queue-5.8 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
Please hold this one back for the moment: we shall want it, but syzbot detected one place where it can lead to a VM_BUG_ON_MM(). The fix to that is currently in Andrew's tree, but not yet in Linus's - when it gets there, I'll send you its git commit id in reply to this mail.
This patch failed to apply to earlier releases: I'll send the fixup for those at that time. (Fixups for another patch to follow later today.)
Now dropped, thanks!
f3f99d63a815 khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter() has now reached Linus's tree, so will reach your tree when you next pull.
When that one is ready, please reinstate this commit that we held back: bbe98f9cadff khugepaged: khugepaged_test_exit() check mmget_still_valid()
The mmap_sem->mmap_lock change means I must then send you a backport of bbe98f9cadff for 5.7, 5.4, 4.19, 4.14, 4.9: one backport will do for all of those, and f3f99d63a815 should cherry-pick cleanly into them all.
But you also marked bbe98f9cadff for 4.4: I had not expected that, but I think you're right - for whatever reason (probably inertia, it was tiresome because khugepaged.c got split from huge_memory.c), 4.4 lacks a backport of 59ea6d06cfa9 (though it does have the commit that depended on), and backports of these two will serve just as well to fix what it was required to fix: I'll send them too.
Thanks: I'm sorry that this is all so confusing, kudos to syzbot for catching my error as quickly as it did.
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
Thanks!
On Sat, 22 Aug 2020, Sasha Levin wrote:
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
That's saved me time, thanks a lot for doing that work, Sasha.
I've checked the results (haha, read on) and they're all fine, but one minor flaw in bisectability: the added 4.4 backport of "coredump: fix race condition..." adds a line (deleted by the next commit) result = SCAN_ANY_PROCESS; but neither "result" nor "SCAN_ANY_PROCESS" is defined in that tree, so that intermediate step would generate an easily fixed build error.
FWIW - I don't know whether that's something to care about or not.
Thanks again, Hugh
On Sat, 22 Aug 2020, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Sasha Levin wrote:
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
That's saved me time, thanks a lot for doing that work, Sasha.
I've checked the results (haha, read on) and they're all fine, but one minor flaw in bisectability: the added 4.4 backport of "coredump: fix race condition..." adds a line (deleted by the next commit) result = SCAN_ANY_PROCESS; but neither "result" nor "SCAN_ANY_PROCESS" is defined in that tree, so that intermediate step would generate an easily fixed build error.
FWIW - I don't know whether that's something to care about or not.
Ah, but I missed that this one that we originally held back from 5.8, did not in fact get re-added to 5.8: all the backport series have it, but today's 5.8.4-rc1 does not have it.
That's not a disaster - the series builds without it, and having its fix without the fixed commit is just odd, no more unsafe than before; but it should be re-added for a 5.8.4-rc2 or 5.8.5.
Thanks, Hugh
From bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 Mon Sep 17 00:00:00 2001
From: Hugh Dickins hughd@google.com Date: Thu, 6 Aug 2020 23:26:25 -0700 Subject: khugepaged: khugepaged_test_exit() check mmget_still_valid()
From: Hugh Dickins hughd@google.com
commit bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 upstream.
Move collapse_huge_page()'s mmget_still_valid() check into khugepaged_test_exit() itself. collapse_huge_page() is used for anon THP only, and earned its mmget_still_valid() check because it inserts a huge pmd entry in place of the page table's pmd entry; whereas collapse_file()'s retract_page_tables() or collapse_pte_mapped_thp() merely clears the page table's pmd entry. But core dumping without mmap lock must have been as open to mistaking a racily cleared pmd entry for a page table at physical page 0, as exit_mmap() was. And we certainly have no interest in mapping as a THP once dumping core.
Fixes: 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") Signed-off-by: Hugh Dickins hughd@google.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Cc: Andrea Arcangeli aarcange@redhat.com Cc: Song Liu songliubraving@fb.com Cc: Mike Kravetz mike.kravetz@oracle.com Cc: Kirill A. Shutemov kirill.shutemov@linux.intel.com Cc: stable@vger.kernel.org [4.8+] Link: http://lkml.kernel.org/r/alpine.LSU.2.11.2008021217020.27773@eggly.anvils Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- mm/khugepaged.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
--- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -431,7 +431,7 @@ static void insert_to_mm_slots_hash(stru
static inline int khugepaged_test_exit(struct mm_struct *mm) { - return atomic_read(&mm->mm_users) == 0; + return atomic_read(&mm->mm_users) == 0 || !mmget_still_valid(mm); }
static bool hugepage_vma_check(struct vm_area_struct *vma, @@ -1100,9 +1100,6 @@ static void collapse_huge_page(struct mm * handled by the anon_vma lock + PG_lock. */ mmap_write_lock(mm); - result = SCAN_ANY_PROCESS; - if (!mmget_still_valid(mm)) - goto out; result = hugepage_vma_revalidate(mm, address, &vma); if (result) goto out;
On Mon, 24 Aug 2020, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Sasha Levin wrote:
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
That's saved me time, thanks a lot for doing that work, Sasha.
I've checked the results (haha, read on) and they're all fine, but one minor flaw in bisectability: the added 4.4 backport of "coredump: fix race condition..." adds a line (deleted by the next commit) result = SCAN_ANY_PROCESS; but neither "result" nor "SCAN_ANY_PROCESS" is defined in that tree, so that intermediate step would generate an easily fixed build error.
FWIW - I don't know whether that's something to care about or not.
Ah, but I missed that this one that we originally held back from 5.8, did not in fact get re-added to 5.8: all the backport series have it, but today's 5.8.4-rc1 does not have it.
That's not a disaster - the series builds without it, and having its fix without the fixed commit is just odd, no more unsafe than before; but it should be re-added for a 5.8.4-rc2 or 5.8.5.
I see 5.8 is at 5.8.5-rc1 today, but the commit below still missing: please re-add it, then we can all forget about it at last - thanks!
Hugh
From bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 Mon Sep 17 00:00:00 2001
From: Hugh Dickins hughd@google.com Date: Thu, 6 Aug 2020 23:26:25 -0700 Subject: khugepaged: khugepaged_test_exit() check mmget_still_valid()
From: Hugh Dickins hughd@google.com
commit bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 upstream.
Move collapse_huge_page()'s mmget_still_valid() check into khugepaged_test_exit() itself. collapse_huge_page() is used for anon THP only, and earned its mmget_still_valid() check because it inserts a huge pmd entry in place of the page table's pmd entry; whereas collapse_file()'s retract_page_tables() or collapse_pte_mapped_thp() merely clears the page table's pmd entry. But core dumping without mmap lock must have been as open to mistaking a racily cleared pmd entry for a page table at physical page 0, as exit_mmap() was. And we certainly have no interest in mapping as a THP once dumping core.
Fixes: 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") Signed-off-by: Hugh Dickins hughd@google.com Signed-off-by: Andrew Morton akpm@linux-foundation.org Cc: Andrea Arcangeli aarcange@redhat.com Cc: Song Liu songliubraving@fb.com Cc: Mike Kravetz mike.kravetz@oracle.com Cc: Kirill A. Shutemov kirill.shutemov@linux.intel.com Cc: stable@vger.kernel.org [4.8+] Link: http://lkml.kernel.org/r/alpine.LSU.2.11.2008021217020.27773@eggly.anvils Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- mm/khugepaged.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
--- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -431,7 +431,7 @@ static void insert_to_mm_slots_hash(stru
static inline int khugepaged_test_exit(struct mm_struct *mm) { - return atomic_read(&mm->mm_users) == 0; + return atomic_read(&mm->mm_users) == 0 || !mmget_still_valid(mm); }
static bool hugepage_vma_check(struct vm_area_struct *vma, @@ -1100,9 +1100,6 @@ static void collapse_huge_page(struct mm * handled by the anon_vma lock + PG_lock. */ mmap_write_lock(mm); - result = SCAN_ANY_PROCESS; - if (!mmget_still_valid(mm)) - goto out; result = hugepage_vma_revalidate(mm, address, &vma); if (result) goto out;
On Wed, Aug 26, 2020 at 11:53:24AM -0700, Hugh Dickins wrote:
On Mon, 24 Aug 2020, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Sasha Levin wrote:
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
That's saved me time, thanks a lot for doing that work, Sasha.
I've checked the results (haha, read on) and they're all fine, but one minor flaw in bisectability: the added 4.4 backport of "coredump: fix race condition..." adds a line (deleted by the next commit) result = SCAN_ANY_PROCESS; but neither "result" nor "SCAN_ANY_PROCESS" is defined in that tree, so that intermediate step would generate an easily fixed build error.
FWIW - I don't know whether that's something to care about or not.
Ah, but I missed that this one that we originally held back from 5.8, did not in fact get re-added to 5.8: all the backport series have it, but today's 5.8.4-rc1 does not have it.
That's not a disaster - the series builds without it, and having its fix without the fixed commit is just odd, no more unsafe than before; but it should be re-added for a 5.8.4-rc2 or 5.8.5.
I see 5.8 is at 5.8.5-rc1 today, but the commit below still missing: please re-add it, then we can all forget about it at last - thanks!
Greg went for a fast release cycle and I didn't have time to queue it up, sorry. But don't worry - this patch isn't forgotten, I'll queue it for the next release on Friday/Saturday.
On Wed, 26 Aug 2020, Sasha Levin wrote:
On Wed, Aug 26, 2020 at 11:53:24AM -0700, Hugh Dickins wrote:
I see 5.8 is at 5.8.5-rc1 today, but the commit below still missing: please re-add it, then we can all forget about it at last - thanks!
Greg went for a fast release cycle and I didn't have time to queue it up, sorry. But don't worry - this patch isn't forgotten, I'll queue it for the next release on Friday/Saturday.
Great, thanks for the reassurance, I'll relax now.
Hugh
On Sat, Aug 22, 2020 at 07:16:39PM -0700, Hugh Dickins wrote:
On Sat, 22 Aug 2020, Sasha Levin wrote:
I've followed your instructions and backported the patches:
bbe98f9cadff ("khugepaged: khugepaged_test_exit() check mmget_still_valid()") - to all branches. f3f99d63a815 ("khugepaged: adjust VM_BUG_ON_MM() in __khugepaged_enter()") - to all branches. 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping") - for 4.4.
That's saved me time, thanks a lot for doing that work, Sasha.
I've checked the results (haha, read on) and they're all fine, but one minor flaw in bisectability: the added 4.4 backport of "coredump: fix race condition..." adds a line (deleted by the next commit) result = SCAN_ANY_PROCESS; but neither "result" nor "SCAN_ANY_PROCESS" is defined in that tree, so that intermediate step would generate an easily fixed build error.
FWIW - I don't know whether that's something to care about or not.
I'd probably prefer leaving it this way rather than dragging in more patches or modifying the existing ones given that it doesn't effect anything beyond bisectability.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Hugh Dickins -
Sasha Levin