[PATCH 0/6] A few fixup patches for hugetlb

[PATCH 0/6] A few fixup patches for hugetlb

[Miaohe Lin]

Hi everyone,
This series contains a few fixup patches to fix incorrect update of page
refcnt, fix possible use-after-free issue and so on. More details can be
found in the respective changelogs.
Thanks!

Miaohe Lin (6):
  mm, hwpoison: fix page refcnt leaking in try_memory_failure_hugetlb()
  mm, hwpoison: fix page refcnt leaking in unpoison_memory()
  mm, hwpoison: fix extra put_page() in soft_offline_page()
  mm, hwpoison: fix possible use-after-free in mf_dax_kill_procs()
  mm, hwpoison: kill procs if unmap fails
  mm, hwpoison: avoid trying to unpoison reserved page

 mm/memory-failure.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

-- 
2.23.0

[PATCH 1/6] mm, hwpoison: fix page refcnt leaking in try_memory_failure_hugetlb()

[Miaohe Lin]

When hwpoison_filter() refuses to hwpoison a hugetlb page, the refcnt of
the page would have been incremented if res == 1. Using put_page() to fix
the refcnt leaking in this case.

Fixes: 405ce051236c ("mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index e48f6f6a259d..22840cd5fe59 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1860,8 +1860,10 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
 
 	if (hwpoison_filter(p)) {
 		hugetlb_clear_page_hwpoison(head);
-		res = -EOPNOTSUPP;
-		goto out;
+		unlock_page(head);
+		if (res == 1)
+			put_page(head);
+		return -EOPNOTSUPP;
 	}
 
 	/*
-- 
2.23.0

[PATCH 2/6] mm, hwpoison: fix page refcnt leaking in unpoison_memory()

[Miaohe Lin]

When free_raw_hwp_pages() fails its work, the refcnt of the hugetlb page
would have been incremented if ret > 0. Using put_page() to fix refcnt
leaking in this case.

Fixes: debb6b9c3fdd ("mm, hwpoison: make unpoison aware of raw error info in hwpoisoned hugepage")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 22840cd5fe59..0c5ad7505b99 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2378,6 +2378,7 @@ int unpoison_memory(unsigned long pfn)
 			count = free_raw_hwp_pages(page, false);
 			if (count == 0) {
 				ret = -EBUSY;
+				put_page(page);
 				goto unlock_mutex;
 			}
 		}
-- 
2.23.0

[PATCH 3/6] mm, hwpoison: fix extra put_page() in soft_offline_page()

[Miaohe Lin]

When hwpoison_filter() refuses to soft offline a page, the page refcnt
incremented previously by MF_COUNT_INCREASED would have been consumed
via get_hwpoison_page() if ret <= 0. So the put_ref_page() here will
put the extra one. Remove it to fix the issue.

Fixes: 9113eaf331bf ("mm/memory-failure.c: add hwpoison_filter for soft offline")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 0c5ad7505b99..7023c3d81273 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2591,8 +2591,6 @@ int soft_offline_page(unsigned long pfn, int flags)
 	if (hwpoison_filter(page)) {
 		if (ret > 0)
 			put_page(page);
-		else
-			put_ref_page(ref_page);
 
 		mutex_unlock(&mf_mutex);
 		return -EOPNOTSUPP;
-- 
2.23.0

[PATCH 4/6] mm, hwpoison: fix possible use-after-free in mf_dax_kill_procs()

[Miaohe Lin]

After kill_procs(), tk will be freed without being removed from the to_kill
list. In the next iteration, the freed list entry in the to_kill list will
be accessed, thus leading to use-after-free issue. Fix it by reinitializing
the to_kill list after unmap_and_kill().

Fixes: c36e20249571 ("mm: introduce mf_dax_kill_procs() for fsdax case")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 7023c3d81273..a2f4e8b00a26 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1658,6 +1658,8 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
 		collect_procs_fsdax(page, mapping, index, &to_kill);
 		unmap_and_kill(&to_kill, page_to_pfn(page), mapping,
 				index, mf_flags);
+		/* Reinitialize to_kill list for later resuing. */
+		INIT_LIST_HEAD(&to_kill);
 unlock:
 		dax_unlock_mapping_entry(mapping, index, cookie);
 	}
-- 
2.23.0

[PATCH 5/6] mm, hwpoison: kill procs if unmap fails

[Miaohe Lin]

If try_to_unmap() fails, the hwpoisoned page still resides in the address
space of some processes. We should kill these processes or the hwpoisoned
page might be consumed later. collect_procs() is always called to collect
relevant processes now so they can be killed later if unmap fails.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index a2f4e8b00a26..5f9615a86296 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1396,7 +1396,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
 	struct address_space *mapping;
 	LIST_HEAD(tokill);
 	bool unmap_success;
-	int kill = 1, forcekill;
+	int forcekill;
 	bool mlocked = PageMlocked(hpage);
 
 	/*
@@ -1437,7 +1437,6 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
 		if (page_mkclean(hpage)) {
 			SetPageDirty(hpage);
 		} else {
-			kill = 0;
 			ttu |= TTU_IGNORE_HWPOISON;
 			pr_info("%#lx: corrupted page was clean: dropped without side effects\n",
 				pfn);
@@ -1452,8 +1451,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
 	 * Error handling: We ignore errors here because
 	 * there's nothing that can be done.
 	 */
-	if (kill)
-		collect_procs(hpage, &tokill, flags & MF_ACTION_REQUIRED);
+	collect_procs(hpage, &tokill, flags & MF_ACTION_REQUIRED);
 
 	if (PageHuge(hpage) && !PageAnon(hpage)) {
 		/*
@@ -1495,7 +1493,8 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
 	 * use a more force-full uncatchable kill to prevent
 	 * any accesses to the poisoned memory.
 	 */
-	forcekill = PageDirty(hpage) || (flags & MF_MUST_KILL);
+	forcekill = PageDirty(hpage) || (flags & MF_MUST_KILL) ||
+		    !unmap_success;
 	kill_procs(&tokill, forcekill, !unmap_success, pfn, flags);
 
 	return unmap_success;
-- 
2.23.0

[PATCH 6/6] mm, hwpoison: avoid trying to unpoison reserved page

[Miaohe Lin]

For reserved pages, HWPoison flag will be set without increasing the page
refcnt. So we shouldn't even try to unpoison these pages and thus decrease
the page refcnt unexpectly. Add a PageReserved() check to filter this case
out and remove the below unneeded zero page (zero page is reserved) check.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 mm/memory-failure.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 5f9615a86296..c831c41bb092 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2355,7 +2355,7 @@ int unpoison_memory(unsigned long pfn)
 		goto unlock_mutex;
 	}
 
-	if (PageSlab(page) || PageTable(page))
+	if (PageSlab(page) || PageTable(page) || PageReserved(page))
 		goto unlock_mutex;
 
 	ret = get_hwpoison_page(p, MF_UNPOISON);
@@ -2386,7 +2386,7 @@ int unpoison_memory(unsigned long pfn)
 		freeit = !!TestClearPageHWPoison(p);
 
 		put_page(page);
-		if (freeit && !(pfn == my_zero_pfn(0) && page_count(p) == 1)) {
+		if (freeit) {
 			put_page(page);
 			ret = 0;
 		}
-- 
2.23.0

Re: [PATCH 0/6] A few fixup patches for hugetlb

[Miaohe Lin]

On 2022/8/18 21:00, Miaohe Lin wrote:
> Hi everyone,
> This series contains a few fixup patches to fix incorrect update of page
> refcnt, fix possible use-after-free issue and so on. More details can be
> found in the respective changelogs.
> Thanks!

Sorry for the wrong patch series cover name. It should be memory-failure instead of hugetlb.

Re: [PATCH 1/6] mm, hwpoison: fix page refcnt leaking in try_memory_failure_hugetlb()

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:11PM +0800, Miaohe Lin wrote:
> When hwpoison_filter() refuses to hwpoison a hugetlb page, the refcnt of
> the page would have been incremented if res == 1. Using put_page() to fix
> the refcnt leaking in this case.
> 
> Fixes: 405ce051236c ("mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Looks good to me, thank you.

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

> ---
>  mm/memory-failure.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index e48f6f6a259d..22840cd5fe59 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1860,8 +1860,10 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags, int *hugetlb
>  
>  	if (hwpoison_filter(p)) {
>  		hugetlb_clear_page_hwpoison(head);
> -		res = -EOPNOTSUPP;
> -		goto out;
> +		unlock_page(head);
> +		if (res == 1)
> +			put_page(head);
> +		return -EOPNOTSUPP;
>  	}
>  
>  	/*
> -- 
> 2.23.0

Re: [PATCH 2/6] mm, hwpoison: fix page refcnt leaking in unpoison_memory()

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:12PM +0800, Miaohe Lin wrote:
> When free_raw_hwp_pages() fails its work, the refcnt of the hugetlb page
> would have been incremented if ret > 0. Using put_page() to fix refcnt
> leaking in this case.
> 
> Fixes: debb6b9c3fdd ("mm, hwpoison: make unpoison aware of raw error info in hwpoisoned hugepage")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

> ---
>  mm/memory-failure.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 22840cd5fe59..0c5ad7505b99 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2378,6 +2378,7 @@ int unpoison_memory(unsigned long pfn)
>  			count = free_raw_hwp_pages(page, false);
>  			if (count == 0) {
>  				ret = -EBUSY;
> +				put_page(page);
>  				goto unlock_mutex;
>  			}
>  		}
> -- 
> 2.23.0

Re: [PATCH 3/6] mm, hwpoison: fix extra put_page() in soft_offline_page()

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:13PM +0800, Miaohe Lin wrote:
> When hwpoison_filter() refuses to soft offline a page, the page refcnt
> incremented previously by MF_COUNT_INCREASED would have been consumed
> via get_hwpoison_page() if ret <= 0. So the put_ref_page() here will
> put the extra one. Remove it to fix the issue.
> 
> Fixes: 9113eaf331bf ("mm/memory-failure.c: add hwpoison_filter for soft offline")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

> ---
>  mm/memory-failure.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 0c5ad7505b99..7023c3d81273 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2591,8 +2591,6 @@ int soft_offline_page(unsigned long pfn, int flags)
>  	if (hwpoison_filter(page)) {
>  		if (ret > 0)
>  			put_page(page);
> -		else
> -			put_ref_page(ref_page);
>  
>  		mutex_unlock(&mf_mutex);
>  		return -EOPNOTSUPP;
> -- 
> 2.23.0

Re: [PATCH 4/6] mm, hwpoison: fix possible use-after-free in mf_dax_kill_procs()

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:14PM +0800, Miaohe Lin wrote:
> After kill_procs(), tk will be freed without being removed from the to_kill
> list. In the next iteration, the freed list entry in the to_kill list will
> be accessed, thus leading to use-after-free issue.

kill_procs() runs over the to_kill list and frees all listed items in each
iteration.  So just after returning from unmap_and_kill(), to_kill->next and
to_kill->prev still point to the addresses of struct to_kill which was the
first or last item (already freed!).  This is bad-manered, but
collect_procs_fsdax() in the next iteration calls list_add_tail() and
overwrites the dangling pointers with newly allocated item.  So this problem
should not be so critical?  Anyway, I agree with fixing this fragile code.

> Fix it by reinitializing
> the to_kill list after unmap_and_kill().
> 
> Fixes: c36e20249571 ("mm: introduce mf_dax_kill_procs() for fsdax case")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

> ---
>  mm/memory-failure.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 7023c3d81273..a2f4e8b00a26 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1658,6 +1658,8 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
>  		collect_procs_fsdax(page, mapping, index, &to_kill);
>  		unmap_and_kill(&to_kill, page_to_pfn(page), mapping,
>  				index, mf_flags);
> +		/* Reinitialize to_kill list for later resuing. */

s/resuing/reusing/ ?

> +		INIT_LIST_HEAD(&to_kill);

How about adding list_del() in kill_procs()?  Other callers now use
to_kill only once, but fixing generally looks tidier to me.

Thanks,
Naoya Horiguchi

Re: [PATCH 5/6] mm, hwpoison: kill procs if unmap fails

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:15PM +0800, Miaohe Lin wrote:
> If try_to_unmap() fails, the hwpoisoned page still resides in the address
> space of some processes. We should kill these processes or the hwpoisoned
> page might be consumed later. collect_procs() is always called to collect
> relevant processes now so they can be killed later if unmap fails.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  mm/memory-failure.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a2f4e8b00a26..5f9615a86296 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1396,7 +1396,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  	struct address_space *mapping;
>  	LIST_HEAD(tokill);
>  	bool unmap_success;
> -	int kill = 1, forcekill;
> +	int forcekill;
>  	bool mlocked = PageMlocked(hpage);
>  
>  	/*
> @@ -1437,7 +1437,6 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  		if (page_mkclean(hpage)) {
>  			SetPageDirty(hpage);
>  		} else {
> -			kill = 0;
>  			ttu |= TTU_IGNORE_HWPOISON;
>  			pr_info("%#lx: corrupted page was clean: dropped without side effects\n",
>  				pfn);
> @@ -1452,8 +1451,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  	 * Error handling: We ignore errors here because
>  	 * there's nothing that can be done.

This above comment might be deprecated now (I'm not sure what this really mean),
so could you drop or update this?

Anyway, the patch looks good to me.

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

>  	 */
> -	if (kill)
> -		collect_procs(hpage, &tokill, flags & MF_ACTION_REQUIRED);
> +	collect_procs(hpage, &tokill, flags & MF_ACTION_REQUIRED);
>  
>  	if (PageHuge(hpage) && !PageAnon(hpage)) {
>  		/*
> @@ -1495,7 +1493,8 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>  	 * use a more force-full uncatchable kill to prevent
>  	 * any accesses to the poisoned memory.
>  	 */
> -	forcekill = PageDirty(hpage) || (flags & MF_MUST_KILL);
> +	forcekill = PageDirty(hpage) || (flags & MF_MUST_KILL) ||
> +		    !unmap_success;
>  	kill_procs(&tokill, forcekill, !unmap_success, pfn, flags);
>  
>  	return unmap_success;
> -- 
> 2.23.0

Re: [PATCH 6/6] mm, hwpoison: avoid trying to unpoison reserved page

[HORIGUCHI NAOYA(堀口 直也)]

On Thu, Aug 18, 2022 at 09:00:16PM +0800, Miaohe Lin wrote:
> For reserved pages, HWPoison flag will be set without increasing the page
> refcnt. So we shouldn't even try to unpoison these pages and thus decrease
> the page refcnt unexpectly. Add a PageReserved() check to filter this case
> out and remove the below unneeded zero page (zero page is reserved) check.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Looks good to me, thank you.

Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

> ---
>  mm/memory-failure.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 5f9615a86296..c831c41bb092 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2355,7 +2355,7 @@ int unpoison_memory(unsigned long pfn)
>  		goto unlock_mutex;
>  	}
>  
> -	if (PageSlab(page) || PageTable(page))
> +	if (PageSlab(page) || PageTable(page) || PageReserved(page))
>  		goto unlock_mutex;
>  
>  	ret = get_hwpoison_page(p, MF_UNPOISON);
> @@ -2386,7 +2386,7 @@ int unpoison_memory(unsigned long pfn)
>  		freeit = !!TestClearPageHWPoison(p);
>  
>  		put_page(page);
> -		if (freeit && !(pfn == my_zero_pfn(0) && page_count(p) == 1)) {
> +		if (freeit) {
>  			put_page(page);
>  			ret = 0;
>  		}
> -- 
> 2.23.0

Re: [PATCH 4/6] mm, hwpoison: fix possible use-after-free in mf_dax_kill_procs()

[Miaohe Lin]

On 2022/8/19 13:23, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Thu, Aug 18, 2022 at 09:00:14PM +0800, Miaohe Lin wrote:
>> After kill_procs(), tk will be freed without being removed from the to_kill
>> list. In the next iteration, the freed list entry in the to_kill list will
>> be accessed, thus leading to use-after-free issue.
> 
> kill_procs() runs over the to_kill list and frees all listed items in each
> iteration.  So just after returning from unmap_and_kill(), to_kill->next and
> to_kill->prev still point to the addresses of struct to_kill which was the
> first or last item (already freed!).  This is bad-manered, but
> collect_procs_fsdax() in the next iteration calls list_add_tail() and
> overwrites the dangling pointers with newly allocated item.  So this problem

list_add_tail will do WRITE_ONCE(prev->next, new) where prev is already freed!
Or am I miss something?

> should not be so critical?  Anyway, I agree with fixing this fragile code.
> 
>> Fix it by reinitializing
>> the to_kill list after unmap_and_kill().
>>
>> Fixes: c36e20249571 ("mm: introduce mf_dax_kill_procs() for fsdax case")
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
>> ---
>>  mm/memory-failure.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index 7023c3d81273..a2f4e8b00a26 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1658,6 +1658,8 @@ int mf_dax_kill_procs(struct address_space *mapping, pgoff_t index,
>>  		collect_procs_fsdax(page, mapping, index, &to_kill);
>>  		unmap_and_kill(&to_kill, page_to_pfn(page), mapping,
>>  				index, mf_flags);
>> +		/* Reinitialize to_kill list for later resuing. */
> 
> s/resuing/reusing/ ?

OK.

> 
>> +		INIT_LIST_HEAD(&to_kill);
> 
> How about adding list_del() in kill_procs()?  Other callers now use
> to_kill only once, but fixing generally looks tidier to me.

That's a good idea. Will do it in v2. Many thanks for your review, Naoya!

Thanks,
Miaohe Lin

> 
> Thanks,
> Naoya Horiguchi
> 

Re: [PATCH 5/6] mm, hwpoison: kill procs if unmap fails

[Miaohe Lin]

On 2022/8/19 13:24, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Thu, Aug 18, 2022 at 09:00:15PM +0800, Miaohe Lin wrote:
>> If try_to_unmap() fails, the hwpoisoned page still resides in the address
>> space of some processes. We should kill these processes or the hwpoisoned
>> page might be consumed later. collect_procs() is always called to collect
>> relevant processes now so they can be killed later if unmap fails.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  mm/memory-failure.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index a2f4e8b00a26..5f9615a86296 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1396,7 +1396,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>>  	struct address_space *mapping;
>>  	LIST_HEAD(tokill);
>>  	bool unmap_success;
>> -	int kill = 1, forcekill;
>> +	int forcekill;
>>  	bool mlocked = PageMlocked(hpage);
>>  
>>  	/*
>> @@ -1437,7 +1437,6 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>>  		if (page_mkclean(hpage)) {
>>  			SetPageDirty(hpage);
>>  		} else {
>> -			kill = 0;
>>  			ttu |= TTU_IGNORE_HWPOISON;
>>  			pr_info("%#lx: corrupted page was clean: dropped without side effects\n",
>>  				pfn);
>> @@ -1452,8 +1451,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
>>  	 * Error handling: We ignore errors here because
>>  	 * there's nothing that can be done.
> 
> This above comment might be deprecated now (I'm not sure what this really mean),
> so could you drop or update this?

Do you mean remove the below comment? In fact, this doesn't make much sense for me.

* Error handling: We ignore errors here because
* there's nothing that can be done.

> 
> Anyway, the patch looks good to me.
> 
> Acked-by: Naoya Horiguchi <naoya.horiguchi@nec.com>

Many thanks for review.

Thanks,
Miaohe Lin

Re: [PATCH 4/6] mm, hwpoison: fix possible use-after-free in mf_dax_kill_procs()

[HORIGUCHI NAOYA(堀口 直也)]

On Fri, Aug 19, 2022 at 03:32:27PM +0800, Miaohe Lin wrote:
> On 2022/8/19 13:23, HORIGUCHI NAOYA(堀口 直也) wrote:
> > On Thu, Aug 18, 2022 at 09:00:14PM +0800, Miaohe Lin wrote:
> >> After kill_procs(), tk will be freed without being removed from the to_kill
> >> list. In the next iteration, the freed list entry in the to_kill list will
> >> be accessed, thus leading to use-after-free issue.
> > 
> > kill_procs() runs over the to_kill list and frees all listed items in each
> > iteration.  So just after returning from unmap_and_kill(), to_kill->next and
> > to_kill->prev still point to the addresses of struct to_kill which was the
> > first or last item (already freed!).  This is bad-manered, but
> > collect_procs_fsdax() in the next iteration calls list_add_tail() and
> > overwrites the dangling pointers with newly allocated item.  So this problem
> 
> list_add_tail will do WRITE_ONCE(prev->next, new) where prev is already freed!
> Or am I miss something?

No, you're right. Thank you for explanation.

Thanks,
Naoya Horiguchi

Re: [PATCH 5/6] mm, hwpoison: kill procs if unmap fails

[HORIGUCHI NAOYA(堀口 直也)]

On Fri, Aug 19, 2022 at 03:37:23PM +0800, Miaohe Lin wrote:
> On 2022/8/19 13:24, HORIGUCHI NAOYA(堀口 直也) wrote:
> > On Thu, Aug 18, 2022 at 09:00:15PM +0800, Miaohe Lin wrote:
> >> If try_to_unmap() fails, the hwpoisoned page still resides in the address
> >> space of some processes. We should kill these processes or the hwpoisoned
> >> page might be consumed later. collect_procs() is always called to collect
> >> relevant processes now so they can be killed later if unmap fails.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> >> ---
> >>  mm/memory-failure.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index a2f4e8b00a26..5f9615a86296 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -1396,7 +1396,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
> >>  	struct address_space *mapping;
> >>  	LIST_HEAD(tokill);
> >>  	bool unmap_success;
> >> -	int kill = 1, forcekill;
> >> +	int forcekill;
> >>  	bool mlocked = PageMlocked(hpage);
> >>  
> >>  	/*
> >> @@ -1437,7 +1437,6 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
> >>  		if (page_mkclean(hpage)) {
> >>  			SetPageDirty(hpage);
> >>  		} else {
> >> -			kill = 0;
> >>  			ttu |= TTU_IGNORE_HWPOISON;
> >>  			pr_info("%#lx: corrupted page was clean: dropped without side effects\n",
> >>  				pfn);
> >> @@ -1452,8 +1451,7 @@ static bool hwpoison_user_mappings(struct page *p, unsigned long pfn,
> >>  	 * Error handling: We ignore errors here because
> >>  	 * there's nothing that can be done.
> > 
> > This above comment might be deprecated now (I'm not sure what this really mean),
> > so could you drop or update this?
> 
> Do you mean remove the below comment? In fact, this doesn't make much sense for me.
> 
> * Error handling: We ignore errors here because
> * there's nothing that can be done.

Yes, that's what I meant.

Thanks,
Naoya Horiguchi

[PATCH 0/6] A few fixup patches for hugetlb

[Miaohe Lin]

Hi everyone,
This series contains a few fixup patches to fix incorrect update of
max_huge_pages, fix WARN_ON(!kobj) in sysfs_create_group() and so on.
More details can be found in the respective changelogs.
Thanks!

Miaohe Lin (6):
  mm/hugetlb: fix incorrect update of max_huge_pages
  mm/hugetlb: fix WARN_ON(!kobj) in sysfs_create_group()
  mm/hugetlb: fix missing call to restore_reserve_on_error()
  mm: hugetlb_vmemmap: add missing smp_wmb() before set_pte_at()
  mm/hugetlb: fix sysfs group leak in hugetlb_unregister_node()
  mm/hugetlb: make detecting shared pte more reliable

 mm/hugetlb.c         | 46 +++++++++++++++++++++++++++-----------------
 mm/hugetlb_vmemmap.c |  5 +++++
 2 files changed, 33 insertions(+), 18 deletions(-)

-- 
2.23.0