Patch "khugepaged: khugepaged_test_exit() check mmget_still_valid()" has been added to the 5.8-stable tree

DISCUSSION Hugh Dickins 3 participants 10 messages Aug 19, 2020 – Aug 26, 2020 1 message not in the archive

Subsystems (3)

Stable branch Memory management THP

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Hugh Dickins

author

2020-08-19 13:32 UTC #1 lore raw

Quoted 8 lines · On Wed, 19 Aug 2020, [email protected] 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;a=summary

Quoted 6 lines

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 <[email protected]> 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

Quoted 68 lines

From bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 Mon Sep 17 00:00:00 2001
From: Hugh Dickins <[email protected]>
Date: Thu, 6 Aug 2020 23:26:25 -0700
Subject: khugepaged: khugepaged_test_exit() check mmget_still_valid()
From: Hugh Dickins <[email protected]>
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 <[email protected]>
Signed-off-by Andrew Morton <[email protected]>
Cc Andrea Arcangeli <[email protected]>
Cc Song Liu <[email protected]>
Cc Mike Kravetz <[email protected]>
Cc Kirill A. Shutemov <[email protected]>
Cc <[email protected]> [4.8+]
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Linus Torvalds <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 mm/khugepaged.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
mm/khugepaged.c +1 -4
@@ -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;
Patches currently in stable-queue which might be from [email protected] 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

Greg KH

maintainer

2020-08-19 13:52 UTC #2 lore raw

Quoted 16 lines · Hugh Dickins

Quoted 8 lines · On Wed, 19 Aug 2020, [email protected] 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;a=summary
Quoted 6 lines
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 <[email protected]> know about it.

Quoted 7 lines

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

Hugh Dickins

author

2020-08-22 01:26 UTC #3 lore raw

Quoted 22 lines · Greg KH

Quoted 21 lines · Hugh Dickins
Quoted 8 lines · On Wed, 19 Aug 2020, [email protected] 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;a=summary
Quoted 6 lines
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 <[email protected]> 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.

Quoted 4 lines

Quoted 2 lines
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

Sasha Levin

maintainer

2020-08-22 21:20 UTC #4 lore raw

Quoted 45 lines · Hugh Dickins

Quoted 22 lines · Greg KH
Quoted 21 lines · Hugh Dickins
Quoted 8 lines · On Wed, 19 Aug 2020, [email protected] 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;a=summary
Quoted 6 lines
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 <[email protected]> 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.
Quoted 4 lines
Quoted 2 lines
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.

Quoted 2 lines

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!

Signature · 2 lines

Thanks,
Sasha

Hugh Dickins

author

2020-08-23 02:17 UTC #5 lore raw

Quoted 9 lines · Sasha Levin

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

Hugh Dickins

author

2020-08-24 15:13 UTC #6 lore raw

Quoted 13 lines · Hugh Dickins

Quoted 9 lines · Sasha Levin
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.

Quoted 8 lines

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 <[email protected]>
Date: Thu, 6 Aug 2020 23:26:25 -0700
Subject: khugepaged: khugepaged_test_exit() check mmget_still_valid()

From: Hugh Dickins <[email protected]>

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 <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Cc Song Liu <[email protected]>

Cc Mike Kravetz <[email protected]>

Cc Kirill A. Shutemov <[email protected]>

Cc <[email protected]> [4.8+]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Linus Torvalds <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 mm/khugepaged.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

mm/khugepaged.c +1 -4

@@ -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;

Hugh Dickins

author

2020-08-26 18:53 UTC #7 lore raw

Quoted 23 lines · Hugh Dickins

Quoted 13 lines · Hugh Dickins
Quoted 9 lines · Sasha Levin
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.
Quoted 8 lines
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.

Quoted 7 lines

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: Hugh Dickins <[email protected]>

commit bbe98f9cadff58cdd6a4acaeba0efa8565dabe65 upstream.

Fixes 59ea6d06cfa9 ("coredump: fix race condition between collapse_huge_page() and core dumping")

Signed-off-by Hugh Dickins <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Cc Song Liu <[email protected]>

Cc Mike Kravetz <[email protected]>

Cc Kirill A. Shutemov <[email protected]>

Cc <[email protected]> [4.8+]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Linus Torvalds <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 mm/khugepaged.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

mm/khugepaged.c +1 -4

@@ -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;

Sasha Levin

maintainer

2020-08-26 21:46 UTC #8 lore raw

Quoted 28 lines · Hugh Dickins

Quoted 27 lines · Hugh Dickins
Quoted 13 lines · Hugh Dickins
Quoted 9 lines · Sasha Levin
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.
Quoted 8 lines
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.

Quoted 6 lines

Quoted 3 lines
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.

Signature · 2 lines

Thanks,
Sasha

Hugh Dickins

author

2020-08-26 21:54 UTC #9 lore raw

Quoted 8 lines · Sasha Levin

Quoted 3 lines · Hugh Dickins
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

Sasha Levin

maintainer

2020-08-24 15:31 UTC #10 lore raw

In reply to Hugh Dickins

Quoted 13 lines · Hugh Dickins

Quoted 9 lines · Sasha Levin
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.

Quoted 8 lines

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.

Signature · 2 lines

Thanks,
Sasha