thp: use is_zero_pfn only after pte_present check

PATCH In review Minchan Kim 3 participants 5 messages Oct 13, 2015

Subsystems (3)

Stable branch Memory management THP

lore

Share & tools

lore

Minchan Kim

author

2015-10-13 02:35 UTC #0 lore raw

PATCH v2 1/1 thp: use is_zero_pfn only after pte_present check

Use is_zero_pfn on pteval only after pte_present check on pteval (It might be better idea to introduce is_zero_pte where checks pte_present first). Otherwise, it could work with swap or migration entry and if pte_pfn's result is equal to zero_pfn by chance, we lose user's data in __collapse_huge_page_copy. So if you're luck, the application is segfaulted and finally you could see below message when the application is exit.

BUG: Bad rss-counter state mm:ffff88007f099300 idx:2 val:3

Cc <[email protected]>

Acked-by Kirill A. Shutemov <[email protected]>

Reviewed-by Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
Hello Greg,

This patch should go to -stable but when you will apply it after merging of linus tree, it will be surely conflicted due to userfaultfd part.

I want to know how to handle it.

Thanks.

 mm/huge_memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

mm/huge_memory.c +2 -1

index 4b06b8db9df2..bbac913f96bc 100644
@@ -2206,7 +2206,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 	for (_pte = pte; _pte < pte+HPAGE_PMD_NR;
 	     _pte++, address += PAGE_SIZE) {
 		pte_t pteval = *_pte;
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none(pteval) || (pte_present(pteval) &&
+				is_zero_pfn(pte_pfn(pteval)))) {
 			if (!userfaultfd_armed(vma) &&
 			    ++none_or_zero <= khugepaged_max_ptes_none)
 				continue;

Signature · 7 lines

1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Greg Kroah-Hartman

maintainer

2015-10-13 05:41 UTC #1 lore raw

Quoted 14 lines · Minchan Kim

Use is_zero_pfn on pteval only after pte_present check on pteval (It might be better idea to introduce is_zero_pte where checks pte_present first). Otherwise, it could work with swap or migration entry and if pte_pfn's result is equal to zero_pfn by chance, we lose user's data in __collapse_huge_page_copy. So if you're luck, the application is segfaulted and finally you could see below message when the application is exit.
BUG: Bad rss-counter state mm:ffff88007f099300 idx:2 val:3
Cc <[email protected]>
Acked-by Kirill A. Shutemov <[email protected]>
Reviewed-by Andrea Arcangeli <[email protected]>
Signed-off-by Minchan Kim <[email protected]>

Quoted 8 lines

---
Hello Greg,
This patch should go to -stable but when you will apply it after merging of linus tree, it will be surely conflicted due to userfaultfd part.
I want to know how to handle it.

You will get an automated email saying it didn't apply and then you provide a backported version. Or you send a properly backported version to [email protected] before then, with the git commit id of the patch in Linus's tree.

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Minchan Kim

author

2015-10-13 06:24 UTC #2 lore raw

Quoted 22 lines · Greg Kroah-Hartman

Quoted 21 lines · Minchan Kim
Use is_zero_pfn on pteval only after pte_present check on pteval (It might be better idea to introduce is_zero_pte where checks pte_present first). Otherwise, it could work with swap or migration entry and if pte_pfn's result is equal to zero_pfn by chance, we lose user's data in __collapse_huge_page_copy. So if you're luck, the application is segfaulted and finally you could see below message when the application is exit.
BUG: Bad rss-counter state mm:ffff88007f099300 idx:2 val:3
Cc <[email protected]>
Acked-by Kirill A. Shutemov <[email protected]>
Reviewed-by Andrea Arcangeli <[email protected]>
Signed-off-by Minchan Kim <[email protected]>
---
Hello Greg,
This patch should go to -stable but when you will apply it after merging of linus tree, it will be surely conflicted due to userfaultfd part.

Quoted 6 lines

Quoted 1 line
I want to know how to handle it.
You will get an automated email saying it didn't apply and then you provide a backported version. Or you send a properly backported version to [email protected] before then, with the git commit id of the patch in Linus's tree.

Okay, I will send a right version when I received automatd email. Thanks.

Vlastimil Babka

Acked-by on v2 1/1

2015-10-13 07:28 UTC #3 lore raw

In reply to Minchan Kim

Quoted 11 lines · Minchan Kim

Use is_zero_pfn on pteval only after pte_present check on pteval (It might be better idea to introduce is_zero_pte where checks pte_present first). Otherwise, it could work with swap or migration entry and if pte_pfn's result is equal to zero_pfn by chance, we lose user's data in __collapse_huge_page_copy. So if you're luck, the application is segfaulted and finally you could see below message when the application is exit.
BUG: Bad rss-counter state mm:ffff88007f099300 idx:2 val:3
Cc <[email protected]>

More specific:

Cc <[email protected]> # 4.1+

Fixes ca0984caa823 ("mm: incorporate zero pages into transparent huge pages")

Quoted 1 line

Acked-by Kirill A. Shutemov <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

Quoted 22 lines

Reviewed-by Andrea Arcangeli <[email protected]>
Signed-off-by Minchan Kim <[email protected]>
---
Hello Greg,
This patch should go to -stable but when you will apply it after merging of linus tree, it will be surely conflicted due to userfaultfd part.
I want to know how to handle it.
Thanks.
  mm/huge_memory.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
mm/huge_memory.c +0 -0
index 4b06b8db9df2..bbac913f96bc 100644

mm/huge_memory.c static int __collapse_huge_page_isolate(struct vm_area_struct *vma, · line 2213

Quoted 9 lines

mm/huge_memory.c +2 -1

@@ -2206,7 +2206,8 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
  	for (_pte = pte; _pte < pte+HPAGE_PMD_NR;
  	     _pte++, address += PAGE_SIZE) {
  		pte_t pteval = *_pte;
-		if (pte_none(pteval) || is_zero_pfn(pte_pfn(pteval))) {
+		if (pte_none(pteval) || (pte_present(pteval) &&
+				is_zero_pfn(pte_pfn(pteval)))) {
  			if (!userfaultfd_armed(vma) &&
  			    ++none_or_zero <= khugepaged_max_ptes_none)
  				continue;

Minchan Kim

author

2015-10-13 09:08 UTC #4 lore raw

Quoted 13 lines · Vlastimil Babka

Quoted 11 lines · Minchan Kim
Use is_zero_pfn on pteval only after pte_present check on pteval (It might be better idea to introduce is_zero_pte where checks pte_present first). Otherwise, it could work with swap or migration entry and if pte_pfn's result is equal to zero_pfn by chance, we lose user's data in __collapse_huge_page_copy. So if you're luck, the application is segfaulted and finally you could see below message when the application is exit.
BUG: Bad rss-counter state mm:ffff88007f099300 idx:2 val:3
Cc <[email protected]>

Quoted 8 lines

More specific:
Cc <[email protected]> # 4.1+
Fixes ca0984caa823 ("mm: incorporate zero pages into transparent
huge pages")
Quoted 1 line
Acked-by Kirill A. Shutemov <[email protected]>
Acked-by Vlastimil Babka <[email protected]>

Thanks for the detail and review, Vlastimil.