[PATCH 1/1] mm/ksm: validate KSM rmap items before hwpoison kill

[PATCH 1/1] mm/ksm: validate KSM rmap items before hwpoison kill

[Longlong Xia]

From: Longlong Xia <xialonglong@kylinos.cn>

collect_procs_ksm() walks the stable-node rmap list and queues an
early kill for every task whose mm appears on the anon_vma chain.

That rmap item can be stale by the time memory failure handles the
poisoned KSM page.  A VMA may have been split, unmapped or remapped
after the rmap item was recorded, so matching only vma->vm_mm can send
SIGBUS with an address that no longer maps the poisoned page.

Check that the saved address still belongs to the VMA and that
page_vma_mapped_walk() still finds the poisoned page there before
adding the task to the kill list.

Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 mm/ksm.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/mm/ksm.c b/mm/ksm.c
index 7d5b76478f0b..bc4b2dd894d8 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -3222,6 +3222,27 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
 }
 
 #ifdef CONFIG_MEMORY_FAILURE
+static bool ksm_rmap_item_mapped(const struct page *page,
+				 struct vm_area_struct *vma,
+				 unsigned long addr)
+{
+	struct page_vma_mapped_walk pvmw = {
+		.pfn = page_to_pfn(page),
+		.nr_pages = 1,
+		.vma = vma,
+		.address = addr,
+		.flags = PVMW_SYNC,
+	};
+
+	if (addr < vma->vm_start || addr >= vma->vm_end)
+		return false;
+	if (!page_vma_mapped_walk(&pvmw))
+		return false;
+	page_vma_mapped_walk_done(&pvmw);
+
+	return true;
+}
+
 /*
  * Collect processes when the error hit an ksm page.
  */
@@ -3237,13 +3258,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
 	if (!stable_node)
 		return;
 	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
+		unsigned long addr = rmap_item->address & PAGE_MASK;
 		struct anon_vma *av = rmap_item->anon_vma;
 
 		anon_vma_lock_read(av);
 		rcu_read_lock();
 		for_each_process(tsk) {
 			struct anon_vma_chain *vmac;
-			unsigned long addr;
 			struct task_struct *t =
 				task_early_kill(tsk, force_early);
 			if (!t)
@@ -3253,7 +3274,9 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
 			{
 				vma = vmac->vma;
 				if (vma->vm_mm == t->mm) {
-					addr = rmap_item->address & PAGE_MASK;
+					if (!ksm_rmap_item_mapped(page, vma,
+								  addr))
+						continue;
 					add_to_kill_ksm(t, page, vma, to_kill,
 							addr);
 				}
-- 
2.43.0

Re: [PATCH 1/1] mm/ksm: validate KSM rmap items before hwpoison kill

[David Hildenbrand (Arm)]

On 8/3/26 17:11, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>
> 
> collect_procs_ksm() walks the stable-node rmap list and queues an
> early kill for every task whose mm appears on the anon_vma chain.
> 
> That rmap item can be stale by the time memory failure handles the
> poisoned KSM page.  A VMA may have been split, unmapped or remapped
> after the rmap item was recorded, so matching only vma->vm_mm can send
> SIGBUS with an address that no longer maps the poisoned page.
> 
> Check that the saved address still belongs to the VMA and that
> page_vma_mapped_walk() still finds the poisoned page there before
> adding the task to the kill list.
> 
> Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> ---
>  mm/ksm.c | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..bc4b2dd894d8 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -3222,6 +3222,27 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
>  }
>  
>  #ifdef CONFIG_MEMORY_FAILURE
> +static bool ksm_rmap_item_mapped(const struct page *page,
> +				 struct vm_area_struct *vma,
> +				 unsigned long addr)

Two tab indent on second parameter line

	struct vm_area_struct *vma, unsigned long addr)

> +{
> +	struct page_vma_mapped_walk pvmw = {
> +		.pfn = page_to_pfn(page),
> +		.nr_pages = 1,
> +		.vma = vma,
> +		.address = addr,
> +		.flags = PVMW_SYNC,
> +	};
> +
> +	if (addr < vma->vm_start || addr >= vma->vm_end)
> +		return false;
> +	if (!page_vma_mapped_walk(&pvmw))
> +		return false;
> +	page_vma_mapped_walk_done(&pvmw);
> +

We have page_mapped_in_vma(). So I wonder whether we can find a way to

1) Modify to just work with KSM (CCing Lorenzo)

Maybe it already does. I'm confused as so often.

Looking at the existing caller collect_procs_anon(), it's really only called
on anon folios. Could it already be called on KSM folios? What would happen
in that case? (does it just work because folio->index is still what we expect)

2) Do the following

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index d7670ba4147bf..7eeb3c336cfe9 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -342,6 +342,27 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 }
 
 #ifdef CONFIG_MEMORY_FAILURE
+static unsigned long page_mapped_in_vma_at_address(const struct page *page,
+               struct vm_area_struct *vma, unsigned long addr)
+{
+       const struct folio *folio = page_folio(page);
+       struct page_vma_mapped_walk pvmw = {
+               .pfn = page_to_pfn(page),
+               .nr_pages = 1,
+               .vma = vma,
+               .address = addr,
+               .flags = PVMW_SYNC,
+       };
+
+       if (addr < vma->vm_start || addr >= vma->vm_end)
+               return -EFAULT;
+       if (!page_vma_mapped_walk(&pvmw))
+               return -EFAULT;
+       page_vma_mapped_walk_done(&pvmw);
+out:
+       return pvmw.address;
+}
+
 /**
  * page_mapped_in_vma - check whether a page is really mapped in a VMA
  * @page: the page to test
@@ -355,21 +376,10 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 unsigned long page_mapped_in_vma(const struct page *page,
                struct vm_area_struct *vma)
 {
-       const struct folio *folio = page_folio(page);
-       struct page_vma_mapped_walk pvmw = {
-               .pfn = page_to_pfn(page),
-               .nr_pages = 1,
-               .vma = vma,
-               .flags = PVMW_SYNC,
-       };
+       const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
 
-       pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
-       if (pvmw.address == -EFAULT)
-               goto out;
-       if (!page_vma_mapped_walk(&pvmw))
+       if (addr == -EFAULT)
                return -EFAULT;
-       page_vma_mapped_walk_done(&pvmw);
-out:
-       return pvmw.address;
+       return page_mapped_in_vma_at_address(page, vma, addr);
 }
 #endif


> +	return true;
> +}
> +
>  /*
>   * Collect processes when the error hit an ksm page.
>   */
> @@ -3237,13 +3258,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>  	if (!stable_node)
>  		return;
>  	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
> +		unsigned long addr = rmap_item->address & PAGE_MASK;

Can be const.

>  		struct anon_vma *av = rmap_item->anon_vma;
>  
>  		anon_vma_lock_read(av);
>  		rcu_read_lock();
>  		for_each_process(tsk) {
>  			struct anon_vma_chain *vmac;
> -			unsigned long addr;
>  			struct task_struct *t =
>  				task_early_kill(tsk, force_early);
>  			if (!t)
> @@ -3253,7 +3274,9 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>  			{
>  				vma = vmac->vma;
>  				if (vma->vm_mm == t->mm) {
> -					addr = rmap_item->address & PAGE_MASK;
> +					if (!ksm_rmap_item_mapped(page, vma,
> +								  addr))

jut put that onto a single line, please: easier to read.

> +						continue;
>  					add_to_kill_ksm(t, page, vma, to_kill,
>  							addr);
>  				}


-- 
Cheers,

David

Re: [PATCH 1/1] mm/ksm: validate KSM rmap items before hwpoison kill

[Longlong Xia]

   Hi David,

   Thanks for the review and for suggesting this approach.

   I will send v2 shortly with your Suggested-by tag.

   Thanks,
   Longlong

在 2026/8/5 20:19, David Hildenbrand (Arm) 写道:
> On 8/3/26 17:11, Longlong Xia wrote:
>> From: Longlong Xia <xialonglong@kylinos.cn>
>>
>> collect_procs_ksm() walks the stable-node rmap list and queues an
>> early kill for every task whose mm appears on the anon_vma chain.
>>
>> That rmap item can be stale by the time memory failure handles the
>> poisoned KSM page.  A VMA may have been split, unmapped or remapped
>> after the rmap item was recorded, so matching only vma->vm_mm can send
>> SIGBUS with an address that no longer maps the poisoned page.
>>
>> Check that the saved address still belongs to the VMA and that
>> page_vma_mapped_walk() still finds the poisoned page there before
>> adding the task to the kill list.
>>
>> Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
>> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
>> ---
>>   mm/ksm.c | 27 +++++++++++++++++++++++++--
>>   1 file changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/ksm.c b/mm/ksm.c
>> index 7d5b76478f0b..bc4b2dd894d8 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -3222,6 +3222,27 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
>>   }
>>   
>>   #ifdef CONFIG_MEMORY_FAILURE
>> +static bool ksm_rmap_item_mapped(const struct page *page,
>> +				 struct vm_area_struct *vma,
>> +				 unsigned long addr)
> Two tab indent on second parameter line
>
> 	struct vm_area_struct *vma, unsigned long addr)
>
>> +{
>> +	struct page_vma_mapped_walk pvmw = {
>> +		.pfn = page_to_pfn(page),
>> +		.nr_pages = 1,
>> +		.vma = vma,
>> +		.address = addr,
>> +		.flags = PVMW_SYNC,
>> +	};
>> +
>> +	if (addr < vma->vm_start || addr >= vma->vm_end)
>> +		return false;
>> +	if (!page_vma_mapped_walk(&pvmw))
>> +		return false;
>> +	page_vma_mapped_walk_done(&pvmw);
>> +
> We have page_mapped_in_vma(). So I wonder whether we can find a way to
>
> 1) Modify to just work with KSM (CCing Lorenzo)
>
> Maybe it already does. I'm confused as so often.
>
> Looking at the existing caller collect_procs_anon(), it's really only called
> on anon folios. Could it already be called on KSM folios? What would happen
> in that case? (does it just work because folio->index is still what we expect)
>
> 2) Do the following
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index d7670ba4147bf..7eeb3c336cfe9 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -342,6 +342,27 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>   }
>   
>   #ifdef CONFIG_MEMORY_FAILURE
> +static unsigned long page_mapped_in_vma_at_address(const struct page *page,
> +               struct vm_area_struct *vma, unsigned long addr)
> +{
> +       const struct folio *folio = page_folio(page);
> +       struct page_vma_mapped_walk pvmw = {
> +               .pfn = page_to_pfn(page),
> +               .nr_pages = 1,
> +               .vma = vma,
> +               .address = addr,
> +               .flags = PVMW_SYNC,
> +       };
> +
> +       if (addr < vma->vm_start || addr >= vma->vm_end)
> +               return -EFAULT;
> +       if (!page_vma_mapped_walk(&pvmw))
> +               return -EFAULT;
> +       page_vma_mapped_walk_done(&pvmw);
> +out:
> +       return pvmw.address;
> +}
> +
>   /**
>    * page_mapped_in_vma - check whether a page is really mapped in a VMA
>    * @page: the page to test
> @@ -355,21 +376,10 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>   unsigned long page_mapped_in_vma(const struct page *page,
>                  struct vm_area_struct *vma)
>   {
> -       const struct folio *folio = page_folio(page);
> -       struct page_vma_mapped_walk pvmw = {
> -               .pfn = page_to_pfn(page),
> -               .nr_pages = 1,
> -               .vma = vma,
> -               .flags = PVMW_SYNC,
> -       };
> +       const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
>   
> -       pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
> -       if (pvmw.address == -EFAULT)
> -               goto out;
> -       if (!page_vma_mapped_walk(&pvmw))
> +       if (addr == -EFAULT)
>                  return -EFAULT;
> -       page_vma_mapped_walk_done(&pvmw);
> -out:
> -       return pvmw.address;
> +       return page_mapped_in_vma_at_address(page, vma, addr);
>   }
>   #endif
>
>
>> +	return true;
>> +}
>> +
>>   /*
>>    * Collect processes when the error hit an ksm page.
>>    */
>> @@ -3237,13 +3258,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>>   	if (!stable_node)
>>   		return;
>>   	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
>> +		unsigned long addr = rmap_item->address & PAGE_MASK;
> Can be const.
>
>>   		struct anon_vma *av = rmap_item->anon_vma;
>>   
>>   		anon_vma_lock_read(av);
>>   		rcu_read_lock();
>>   		for_each_process(tsk) {
>>   			struct anon_vma_chain *vmac;
>> -			unsigned long addr;
>>   			struct task_struct *t =
>>   				task_early_kill(tsk, force_early);
>>   			if (!t)
>> @@ -3253,7 +3274,9 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>>   			{
>>   				vma = vmac->vma;
>>   				if (vma->vm_mm == t->mm) {
>> -					addr = rmap_item->address & PAGE_MASK;
>> +					if (!ksm_rmap_item_mapped(page, vma,
>> +								  addr))
> jut put that onto a single line, please: easier to read.
>
>> +						continue;
>>   					add_to_kill_ksm(t, page, vma, to_kill,
>>   							addr);
>>   				}
>

Re: [PATCH 1/1] mm/ksm: validate KSM rmap items before hwpoison kill

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 12:21:24AM +0800, Longlong Xia wrote:
>   Hi David,
>
>   Thanks for the review and for suggesting this approach.
>
>   I will send v2 shortly with your Suggested-by tag.

Hang on :) I wanted to look at this (always better to wait at least a day before
respins in general).

>
>   Thanks,
>   Longlong
>
> 在 2026/8/5 20:19, David Hildenbrand (Arm) 写道:
> > On 8/3/26 17:11, Longlong Xia wrote:
> > > From: Longlong Xia <xialonglong@kylinos.cn>
> > >
> > > collect_procs_ksm() walks the stable-node rmap list and queues an
> > > early kill for every task whose mm appears on the anon_vma chain.
> > >
> > > That rmap item can be stale by the time memory failure handles the
> > > poisoned KSM page.  A VMA may have been split, unmapped or remapped
> > > after the rmap item was recorded, so matching only vma->vm_mm can send
> > > SIGBUS with an address that no longer maps the poisoned page.
> > >
> > > Check that the saved address still belongs to the VMA and that
> > > page_vma_mapped_walk() still finds the poisoned page there before
> > > adding the task to the kill list.
> > >
> > > Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
> > > Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> > > ---
> > >   mm/ksm.c | 27 +++++++++++++++++++++++++--
> > >   1 file changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/ksm.c b/mm/ksm.c
> > > index 7d5b76478f0b..bc4b2dd894d8 100644
> > > --- a/mm/ksm.c
> > > +++ b/mm/ksm.c
> > > @@ -3222,6 +3222,27 @@ void rmap_walk_ksm(struct folio *folio, struct rmap_walk_control *rwc)
> > >   }
> > >   #ifdef CONFIG_MEMORY_FAILURE
> > > +static bool ksm_rmap_item_mapped(const struct page *page,
> > > +				 struct vm_area_struct *vma,
> > > +				 unsigned long addr)
> > Two tab indent on second parameter line
> >
> > 	struct vm_area_struct *vma, unsigned long addr)
> >
> > > +{
> > > +	struct page_vma_mapped_walk pvmw = {
> > > +		.pfn = page_to_pfn(page),
> > > +		.nr_pages = 1,
> > > +		.vma = vma,
> > > +		.address = addr,
> > > +		.flags = PVMW_SYNC,
> > > +	};
> > > +
> > > +	if (addr < vma->vm_start || addr >= vma->vm_end)
> > > +		return false;
> > > +	if (!page_vma_mapped_walk(&pvmw))
> > > +		return false;
> > > +	page_vma_mapped_walk_done(&pvmw);
> > > +
> > We have page_mapped_in_vma(). So I wonder whether we can find a way to
> >
> > 1) Modify to just work with KSM (CCing Lorenzo)
> >
> > Maybe it already does. I'm confused as so often.
> >
> > Looking at the existing caller collect_procs_anon(), it's really only called
> > on anon folios. Could it already be called on KSM folios? What would happen
> > in that case? (does it just work because folio->index is still what we expect)
> >
> > 2) Do the following
> >
> > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> > index d7670ba4147bf..7eeb3c336cfe9 100644
> > --- a/mm/page_vma_mapped.c
> > +++ b/mm/page_vma_mapped.c
> > @@ -342,6 +342,27 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> >   }
> >   #ifdef CONFIG_MEMORY_FAILURE
> > +static unsigned long page_mapped_in_vma_at_address(const struct page *page,
> > +               struct vm_area_struct *vma, unsigned long addr)
> > +{
> > +       const struct folio *folio = page_folio(page);
> > +       struct page_vma_mapped_walk pvmw = {
> > +               .pfn = page_to_pfn(page),
> > +               .nr_pages = 1,
> > +               .vma = vma,
> > +               .address = addr,
> > +               .flags = PVMW_SYNC,
> > +       };
> > +
> > +       if (addr < vma->vm_start || addr >= vma->vm_end)
> > +               return -EFAULT;
> > +       if (!page_vma_mapped_walk(&pvmw))
> > +               return -EFAULT;
> > +       page_vma_mapped_walk_done(&pvmw);
> > +out:
> > +       return pvmw.address;
> > +}
> > +
> >   /**
> >    * page_mapped_in_vma - check whether a page is really mapped in a VMA
> >    * @page: the page to test
> > @@ -355,21 +376,10 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
> >   unsigned long page_mapped_in_vma(const struct page *page,
> >                  struct vm_area_struct *vma)
> >   {
> > -       const struct folio *folio = page_folio(page);
> > -       struct page_vma_mapped_walk pvmw = {
> > -               .pfn = page_to_pfn(page),
> > -               .nr_pages = 1,
> > -               .vma = vma,
> > -               .flags = PVMW_SYNC,
> > -       };
> > +       const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
> > -       pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
> > -       if (pvmw.address == -EFAULT)
> > -               goto out;
> > -       if (!page_vma_mapped_walk(&pvmw))
> > +       if (addr == -EFAULT)
> >                  return -EFAULT;
> > -       page_vma_mapped_walk_done(&pvmw);
> > -out:
> > -       return pvmw.address;
> > +       return page_mapped_in_vma_at_address(page, vma, addr);
> >   }
> >   #endif
> >
> >
> > > +	return true;
> > > +}
> > > +
> > >   /*
> > >    * Collect processes when the error hit an ksm page.
> > >    */
> > > @@ -3237,13 +3258,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
> > >   	if (!stable_node)
> > >   		return;
> > >   	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
> > > +		unsigned long addr = rmap_item->address & PAGE_MASK;
> > Can be const.
> >
> > >   		struct anon_vma *av = rmap_item->anon_vma;
> > >   		anon_vma_lock_read(av);
> > >   		rcu_read_lock();
> > >   		for_each_process(tsk) {
> > >   			struct anon_vma_chain *vmac;
> > > -			unsigned long addr;
> > >   			struct task_struct *t =
> > >   				task_early_kill(tsk, force_early);
> > >   			if (!t)
> > > @@ -3253,7 +3274,9 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
> > >   			{
> > >   				vma = vmac->vma;
> > >   				if (vma->vm_mm == t->mm) {
> > > -					addr = rmap_item->address & PAGE_MASK;
> > > +					if (!ksm_rmap_item_mapped(page, vma,
> > > +								  addr))
> > jut put that onto a single line, please: easier to read.
> >
> > > +						continue;
> > >   					add_to_kill_ksm(t, page, vma, to_kill,
> > >   							addr);
> > >   				}
> >
>

--
Cheers, Lorenzo

[PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[Longlong Xia]

From: Longlong Xia <xialonglong@kylinos.cn>

collect_procs_ksm() walks the stable-node rmap list and queues an
early kill for every task whose mm appears on the anon_vma chain.

That rmap item can be stale by the time memory failure handles the
poisoned KSM page. A VMA may have been split, unmapped or remapped
after the rmap item was recorded, so matching only vma->vm_mm can send
SIGBUS with an address that no longer maps the poisoned page.

Factor page_mapped_in_vma_at_address() out of page_mapped_in_vma() so
callers that already know the virtual address can validate it directly.
This avoids deriving the address from page_pgoff(), which is invalid
for KSM pages. Use the address saved in the KSM rmap item to check that
it still belongs to the VMA and that page_vma_mapped_walk() still finds
the poisoned page before adding the task to the kill list.

Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
---
 mm/internal.h        |  2 ++
 mm/ksm.c             | 10 +++++++---
 mm/page_vma_mapped.c | 36 +++++++++++++++++++++++-------------
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 181e79f1d6a2..4c9e601b2d95 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1424,6 +1424,8 @@ void add_to_kill_ksm(struct task_struct *tsk, const struct page *p,
 		     unsigned long ksm_addr);
 unsigned long page_mapped_in_vma(const struct page *page,
 		struct vm_area_struct *vma);
+unsigned long page_mapped_in_vma_at_address(const struct page *page,
+		struct vm_area_struct *vma, unsigned long addr);
 
 #else
 static inline int unmap_poisoned_folio(struct folio *folio, unsigned long pfn, bool must_kill)
diff --git a/mm/ksm.c b/mm/ksm.c
index 7d5b76478f0b..5104e442fcb2 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -3237,13 +3237,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
 	if (!stable_node)
 		return;
 	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
+		const unsigned long addr = rmap_item->address & PAGE_MASK;
 		struct anon_vma *av = rmap_item->anon_vma;
 
 		anon_vma_lock_read(av);
 		rcu_read_lock();
 		for_each_process(tsk) {
 			struct anon_vma_chain *vmac;
-			unsigned long addr;
 			struct task_struct *t =
 				task_early_kill(tsk, force_early);
 			if (!t)
@@ -3253,9 +3253,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
 			{
 				vma = vmac->vma;
 				if (vma->vm_mm == t->mm) {
-					addr = rmap_item->address & PAGE_MASK;
+					const unsigned long mapped_addr =
+						page_mapped_in_vma_at_address(page, vma, addr);
+
+					if (mapped_addr == -EFAULT)
+						continue;
 					add_to_kill_ksm(t, page, vma, to_kill,
-							addr);
+							mapped_addr);
 				}
 			}
 		}
diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index bac2eb5de63d..f7c5dc9248bc 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -336,6 +336,26 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 }
 
 #ifdef CONFIG_MEMORY_FAILURE
+unsigned long page_mapped_in_vma_at_address(const struct page *page,
+		struct vm_area_struct *vma, unsigned long addr)
+{
+	struct page_vma_mapped_walk pvmw = {
+		.pfn = page_to_pfn(page),
+		.nr_pages = 1,
+		.vma = vma,
+		.address = addr,
+		.flags = PVMW_SYNC,
+	};
+
+	if (addr < vma->vm_start || addr >= vma->vm_end)
+		return -EFAULT;
+	if (!page_vma_mapped_walk(&pvmw))
+		return -EFAULT;
+	page_vma_mapped_walk_done(&pvmw);
+
+	return pvmw.address;
+}
+
 /**
  * page_mapped_in_vma - check whether a page is really mapped in a VMA
  * @page: the page to test
@@ -350,20 +370,10 @@ unsigned long page_mapped_in_vma(const struct page *page,
 		struct vm_area_struct *vma)
 {
 	const struct folio *folio = page_folio(page);
-	struct page_vma_mapped_walk pvmw = {
-		.pfn = page_to_pfn(page),
-		.nr_pages = 1,
-		.vma = vma,
-		.flags = PVMW_SYNC,
-	};
+	const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
 
-	pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
-	if (pvmw.address == -EFAULT)
-		goto out;
-	if (!page_vma_mapped_walk(&pvmw))
+	if (addr == -EFAULT)
 		return -EFAULT;
-	page_vma_mapped_walk_done(&pvmw);
-out:
-	return pvmw.address;
+	return page_mapped_in_vma_at_address(page, vma, addr);
 }
 #endif
-- 
2.43.0

Re: [PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[Lorenzo Stoakes (ARM)]

:))) I did just say don't send a v2 but I guess you didn't see it.

Basics:

- Please don't respin when somebody's literally asked for feedback from somebody
  else.

- Please don't send v2 of a patch in-reply-to a v1 send it separately.

- Please don't send a v2 on the same day as a v1.

- Attach a changelog under the --- with a link to prior versions (using b4 makes
  this easy).

Anyway I guess I am forced to reply here... *grumble*.

Thanks, Lorenzo

On Thu, Aug 06, 2026 at 12:29:37AM +0800, Longlong Xia wrote:
> From: Longlong Xia <xialonglong@kylinos.cn>
>
> collect_procs_ksm() walks the stable-node rmap list and queues an
> early kill for every task whose mm appears on the anon_vma chain.
>
> That rmap item can be stale by the time memory failure handles the
> poisoned KSM page. A VMA may have been split, unmapped or remapped
> after the rmap item was recorded, so matching only vma->vm_mm can send
> SIGBUS with an address that no longer maps the poisoned page.

I'm very confused as to where memory poisoning comes into it? What exactly
made you aware of this? Do you have a bug report you've observed?

>
> Factor page_mapped_in_vma_at_address() out of page_mapped_in_vma() so
> callers that already know the virtual address can validate it directly.
> This avoids deriving the address from page_pgoff(), which is invalid
> for KSM pages. Use the address saved in the KSM rmap item to check that
> it still belongs to the VMA and that page_vma_mapped_walk() still finds
> the poisoned page before adding the task to the kill list.
>
> Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>

Honestly I have to ask - is this your own work or AI-generated? As I'm not
confident you really understand this and it's tricky stuff so if a
backportable patch is in the works I'd prefer somebody who understands it
contributes it.

> ---
>  mm/internal.h        |  2 ++
>  mm/ksm.c             | 10 +++++++---
>  mm/page_vma_mapped.c | 36 +++++++++++++++++++++++-------------
>  3 files changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 181e79f1d6a2..4c9e601b2d95 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1424,6 +1424,8 @@ void add_to_kill_ksm(struct task_struct *tsk, const struct page *p,
>  		     unsigned long ksm_addr);
>  unsigned long page_mapped_in_vma(const struct page *page,
>  		struct vm_area_struct *vma);
> +unsigned long page_mapped_in_vma_at_address(const struct page *page,
> +		struct vm_area_struct *vma, unsigned long addr);
>
>  #else
>  static inline int unmap_poisoned_folio(struct folio *folio, unsigned long pfn, bool must_kill)
> diff --git a/mm/ksm.c b/mm/ksm.c
> index 7d5b76478f0b..5104e442fcb2 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
> @@ -3237,13 +3237,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>  	if (!stable_node)
>  		return;
>  	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
> +		const unsigned long addr = rmap_item->address & PAGE_MASK;
>  		struct anon_vma *av = rmap_item->anon_vma;
>
>  		anon_vma_lock_read(av);
>  		rcu_read_lock();
>  		for_each_process(tsk) {
>  			struct anon_vma_chain *vmac;
> -			unsigned long addr;
>  			struct task_struct *t =
>  				task_early_kill(tsk, force_early);
>  			if (!t)
> @@ -3253,9 +3253,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>  			{

OK so you're literally doing an anon rmap walk here, with the anon lock held.

>  				vma = vmac->vma;
>  				if (vma->vm_mm == t->mm) {
> -					addr = rmap_item->address & PAGE_MASK;
> +					const unsigned long mapped_addr =
> +						page_mapped_in_vma_at_address(page, vma, addr);

Now you're doing another anon rmap walk? Why on earth are you doing that? And
won't this deadlock?

Why aren't you just checking the whether addr is contained in the range here?

Like:

	/* Make sure VMA wasn't split/remapped */
	if (!in_range(addr, vma->vm_start, vma_pages(vma)))
		continue;

Or something?

> +
> +					if (mapped_addr == -EFAULT)
> +						continue;
>  					add_to_kill_ksm(t, page, vma, to_kill,
> -							addr);
> +							mapped_addr);
>  				}
>  			}
>  		}
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index bac2eb5de63d..f7c5dc9248bc 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -336,6 +336,26 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>  }
>
>  #ifdef CONFIG_MEMORY_FAILURE
> +unsigned long page_mapped_in_vma_at_address(const struct page *page,
> +		struct vm_area_struct *vma, unsigned long addr)
> +{
> +	struct page_vma_mapped_walk pvmw = {
> +		.pfn = page_to_pfn(page),
> +		.nr_pages = 1,
> +		.vma = vma,
> +		.address = addr,
> +		.flags = PVMW_SYNC,
> +	};
> +
> +	if (addr < vma->vm_start || addr >= vma->vm_end)
> +		return -EFAULT;
> +	if (!page_vma_mapped_walk(&pvmw))
> +		return -EFAULT;
> +	page_vma_mapped_walk_done(&pvmw);
> +
> +	return pvmw.address;
> +}

I hate this name I hate that it's CONFIG_MEMORY_FAILURE only.

Also it sounds like a predicate but returns an address? I have no idea what this
is supposed to do? And no kdoc?...

> +
>  /**
>   * page_mapped_in_vma - check whether a page is really mapped in a VMA
>   * @page: the page to test
> @@ -350,20 +370,10 @@ unsigned long page_mapped_in_vma(const struct page *page,
>  		struct vm_area_struct *vma)
>  {
>  	const struct folio *folio = page_folio(page);
> -	struct page_vma_mapped_walk pvmw = {
> -		.pfn = page_to_pfn(page),
> -		.nr_pages = 1,
> -		.vma = vma,
> -		.flags = PVMW_SYNC,
> -	};
> +	const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
>
> -	pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
> -	if (pvmw.address == -EFAULT)
> -		goto out;
> -	if (!page_vma_mapped_walk(&pvmw))
> +	if (addr == -EFAULT)
>  		return -EFAULT;
> -	page_vma_mapped_walk_done(&pvmw);
> -out:
> -	return pvmw.address;
> +	return page_mapped_in_vma_at_address(page, vma, addr);

Oh yes, make the !CONFIG_MEMORY_FAILURE build break *eye roll*

>  }
>  #endif
> --
> 2.43.0
>

--
Cheers, Lorenzo

Re: [PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[Longlong Xia]

Hi Lorenzo,

Sorry, you are right. I messed up the process here -- I should not have
resent it so quickly, or sent v2 as a reply to v1.

I did use AI tooling while working on this, but the patch is my
responsibility. I did just follow the anonymous-page handling here, without
thinking it carefully enough.

Thanks,
Longlong


在 2026/8/6 0:47, Lorenzo Stoakes (ARM) 写道:
> :))) I did just say don't send a v2 but I guess you didn't see it.
>
> Basics:
>
> - Please don't respin when somebody's literally asked for feedback from somebody
>    else.
>
> - Please don't send v2 of a patch in-reply-to a v1 send it separately.
>
> - Please don't send a v2 on the same day as a v1.
>
> - Attach a changelog under the --- with a link to prior versions (using b4 makes
>    this easy).
>
> Anyway I guess I am forced to reply here... *grumble*.
>
> Thanks, Lorenzo
>
> On Thu, Aug 06, 2026 at 12:29:37AM +0800, Longlong Xia wrote:
>> From: Longlong Xia <xialonglong@kylinos.cn>
>>
>> collect_procs_ksm() walks the stable-node rmap list and queues an
>> early kill for every task whose mm appears on the anon_vma chain.
>>
>> That rmap item can be stale by the time memory failure handles the
>> poisoned KSM page. A VMA may have been split, unmapped or remapped
>> after the rmap item was recorded, so matching only vma->vm_mm can send
>> SIGBUS with an address that no longer maps the poisoned page.
> I'm very confused as to where memory poisoning comes into it? What exactly
> made you aware of this? Do you have a bug report you've observed?
>
>> Factor page_mapped_in_vma_at_address() out of page_mapped_in_vma() so
>> callers that already know the virtual address can validate it directly.
>> This avoids deriving the address from page_pgoff(), which is invalid
>> for KSM pages. Use the address saved in the KSM rmap item to check that
>> it still belongs to the VMA and that page_vma_mapped_walk() still finds
>> the poisoned page before adding the task to the kill list.
>>
>> Fixes: 4248d0083ec5 ("mm: ksm: support hwpoison for ksm page")
>> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
>> Signed-off-by: Longlong Xia <xialonglong@kylinos.cn>
> Honestly I have to ask - is this your own work or AI-generated? As I'm not
> confident you really understand this and it's tricky stuff so if a
> backportable patch is in the works I'd prefer somebody who understands it
> contributes it.
>
>> ---
>>   mm/internal.h        |  2 ++
>>   mm/ksm.c             | 10 +++++++---
>>   mm/page_vma_mapped.c | 36 +++++++++++++++++++++++-------------
>>   3 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 181e79f1d6a2..4c9e601b2d95 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -1424,6 +1424,8 @@ void add_to_kill_ksm(struct task_struct *tsk, const struct page *p,
>>   		     unsigned long ksm_addr);
>>   unsigned long page_mapped_in_vma(const struct page *page,
>>   		struct vm_area_struct *vma);
>> +unsigned long page_mapped_in_vma_at_address(const struct page *page,
>> +		struct vm_area_struct *vma, unsigned long addr);
>>
>>   #else
>>   static inline int unmap_poisoned_folio(struct folio *folio, unsigned long pfn, bool must_kill)
>> diff --git a/mm/ksm.c b/mm/ksm.c
>> index 7d5b76478f0b..5104e442fcb2 100644
>> --- a/mm/ksm.c
>> +++ b/mm/ksm.c
>> @@ -3237,13 +3237,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>>   	if (!stable_node)
>>   		return;
>>   	hlist_for_each_entry(rmap_item, &stable_node->hlist, hlist) {
>> +		const unsigned long addr = rmap_item->address & PAGE_MASK;
>>   		struct anon_vma *av = rmap_item->anon_vma;
>>
>>   		anon_vma_lock_read(av);
>>   		rcu_read_lock();
>>   		for_each_process(tsk) {
>>   			struct anon_vma_chain *vmac;
>> -			unsigned long addr;
>>   			struct task_struct *t =
>>   				task_early_kill(tsk, force_early);
>>   			if (!t)
>> @@ -3253,9 +3253,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>>   			{
> OK so you're literally doing an anon rmap walk here, with the anon lock held.
>
>>   				vma = vmac->vma;
>>   				if (vma->vm_mm == t->mm) {
>> -					addr = rmap_item->address & PAGE_MASK;
>> +					const unsigned long mapped_addr =
>> +						page_mapped_in_vma_at_address(page, vma, addr);
> Now you're doing another anon rmap walk? Why on earth are you doing that? And
> won't this deadlock?
>
> Why aren't you just checking the whether addr is contained in the range here?
>
> Like:
>
> 	/* Make sure VMA wasn't split/remapped */
> 	if (!in_range(addr, vma->vm_start, vma_pages(vma)))
> 		continue;
>
> Or something?
>
>> +
>> +					if (mapped_addr == -EFAULT)
>> +						continue;
>>   					add_to_kill_ksm(t, page, vma, to_kill,
>> -							addr);
>> +							mapped_addr);
>>   				}
>>   			}
>>   		}
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index bac2eb5de63d..f7c5dc9248bc 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -336,6 +336,26 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>>   }
>>
>>   #ifdef CONFIG_MEMORY_FAILURE
>> +unsigned long page_mapped_in_vma_at_address(const struct page *page,
>> +		struct vm_area_struct *vma, unsigned long addr)
>> +{
>> +	struct page_vma_mapped_walk pvmw = {
>> +		.pfn = page_to_pfn(page),
>> +		.nr_pages = 1,
>> +		.vma = vma,
>> +		.address = addr,
>> +		.flags = PVMW_SYNC,
>> +	};
>> +
>> +	if (addr < vma->vm_start || addr >= vma->vm_end)
>> +		return -EFAULT;
>> +	if (!page_vma_mapped_walk(&pvmw))
>> +		return -EFAULT;
>> +	page_vma_mapped_walk_done(&pvmw);
>> +
>> +	return pvmw.address;
>> +}
> I hate this name I hate that it's CONFIG_MEMORY_FAILURE only.
>
> Also it sounds like a predicate but returns an address? I have no idea what this
> is supposed to do? And no kdoc?...
>
>> +
>>   /**
>>    * page_mapped_in_vma - check whether a page is really mapped in a VMA
>>    * @page: the page to test
>> @@ -350,20 +370,10 @@ unsigned long page_mapped_in_vma(const struct page *page,
>>   		struct vm_area_struct *vma)
>>   {
>>   	const struct folio *folio = page_folio(page);
>> -	struct page_vma_mapped_walk pvmw = {
>> -		.pfn = page_to_pfn(page),
>> -		.nr_pages = 1,
>> -		.vma = vma,
>> -		.flags = PVMW_SYNC,
>> -	};
>> +	const unsigned long addr = vma_address(vma, page_pgoff(folio, page), 1);
>>
>> -	pvmw.address = vma_address(vma, page_pgoff(folio, page), 1);
>> -	if (pvmw.address == -EFAULT)
>> -		goto out;
>> -	if (!page_vma_mapped_walk(&pvmw))
>> +	if (addr == -EFAULT)
>>   		return -EFAULT;
>> -	page_vma_mapped_walk_done(&pvmw);
>> -out:
>> -	return pvmw.address;
>> +	return page_mapped_in_vma_at_address(page, vma, addr);
> Oh yes, make the !CONFIG_MEMORY_FAILURE build break *eye roll*
>
>>   }
>>   #endif
>> --
>> 2.43.0
>>
> --
> Cheers, Lorenzo

Re: [PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 03:48:35PM +0800, Longlong Xia wrote:
> Hi Lorenzo,
>
> Sorry, you are right. I messed up the process here -- I should not have
> resent it so quickly, or sent v2 as a reply to v1.
>
> I did use AI tooling while working on this, but the patch is my
> responsibility. I did just follow the anonymous-page handling here, without
> thinking it carefully enough.

Please include the Assisted-by tag in future revisions if it formed a
substantial part of the patch, as per
https://docs.kernel.org/process/coding-assistants.html thanks!

(Be handy to stick a # < what you used the LLM for > comment after it too :)

>
> Thanks,
> Longlong

--
Cheers, Lorenzo

Re: [PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[Longlong Xia]

I will

Thanks,
Longlong

在 2026/8/6 17:16, Lorenzo Stoakes (ARM) 写道:
> On Thu, Aug 06, 2026 at 03:48:35PM +0800, Longlong Xia wrote:
>> Hi Lorenzo,
>>
>> Sorry, you are right. I messed up the process here -- I should not have
>> resent it so quickly, or sent v2 as a reply to v1.
>>
>> I did use AI tooling while working on this, but the patch is my
>> responsibility. I did just follow the anonymous-page handling here, without
>> thinking it carefully enough.
> Please include the Assisted-by tag in future revisions if it formed a
> substantial part of the patch, as per
> https://docs.kernel.org/process/coding-assistants.html thanks!
>
> (Be handy to stick a # < what you used the LLM for > comment after it too :)
>
>> Thanks,
>> Longlong
> --
> Cheers, Lorenzo

Re: [PATCH v2] mm/ksm: validate KSM rmap items before hwpoison kill

[David Hildenbrand (Arm)]

>> @@ -3253,9 +3253,13 @@ void collect_procs_ksm(const struct folio *folio, const struct page *page,
>>  			{
> 
> OK so you're literally doing an anon rmap walk here, with the anon lock held.
> 
>>  				vma = vmac->vma;
>>  				if (vma->vm_mm == t->mm) {
>> -					addr = rmap_item->address & PAGE_MASK;
>> +					const unsigned long mapped_addr =
>> +						page_mapped_in_vma_at_address(page, vma, addr);
> 
> Now you're doing another anon rmap walk? Why on earth are you doing that? And
> won't this deadlock?

I think it's the same as with page_mapped_in_vma(): it expects the anon vma lock
to already haven been taken.

At least that's what I understand when looking at collect_procs_anon. :)

> 
> Why aren't you just checking the whether addr is contained in the range here?

Yeah, that should be much easier.

> 
> Like:
> 
> 	/* Make sure VMA wasn't split/remapped */
> 	if (!in_range(addr, vma->vm_start, vma_pages(vma)))
> 		continue;
> 
> Or something?
> 
>> +
>> +					if (mapped_addr == -EFAULT)
>> +						continue;
>>  					add_to_kill_ksm(t, page, vma, to_kill,
>> -							addr);
>> +							mapped_addr);
>>  				}
>>  			}
>>  		}
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index bac2eb5de63d..f7c5dc9248bc 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -336,6 +336,26 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>>  }
>>
>>  #ifdef CONFIG_MEMORY_FAILURE
>> +unsigned long page_mapped_in_vma_at_address(const struct page *page,
>> +		struct vm_area_struct *vma, unsigned long addr)
>> +{
>> +	struct page_vma_mapped_walk pvmw = {
>> +		.pfn = page_to_pfn(page),
>> +		.nr_pages = 1,
>> +		.vma = vma,
>> +		.address = addr,
>> +		.flags = PVMW_SYNC,
>> +	};
>> +
>> +	if (addr < vma->vm_start || addr >= vma->vm_end)
>> +		return -EFAULT;
>> +	if (!page_vma_mapped_walk(&pvmw))
>> +		return -EFAULT;
>> +	page_vma_mapped_walk_done(&pvmw);
>> +
>> +	return pvmw.address;
>> +}
> 
> I hate this name I hate that it's CONFIG_MEMORY_FAILURE only.

Note that page_mapped_in_vma() is also KSM only.

But I am curious why the function differs from what I quickly hacked together.


-- 
Cheers,

David