[PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[liqiong]

Upstream has no this bug.

The two functions look up a slot and dereference the pointer,
If the pointer is null, the kernel would crash and dump.

The 'numad' service calls 'migrate_pages' periodically. If some slots
being replaced (Cache Eviction), the radix_tree_lookup_slot() returns
a null pointer, then, kernel crash.

"numad":  crash> bt
[exception RIP: migrate_page_move_mapping+337]

Introduct a pointer checking to avoid dereference a null pointer.

Cc: <stable@vger.kernel.org> # v4.19-rc8
Signed-off-by: liqiong <liqiong@nfschina.com>
---
 mm/migrate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mm/migrate.c b/mm/migrate.c
index 84381b55b2bd..1ff95c259511 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -474,6 +474,10 @@ int migrate_page_move_mapping(struct address_space *mapping,
 
 	pslot = radix_tree_lookup_slot(&mapping->i_pages,
  					page_index(page));
+	if (pslot == NULL) {
+		xa_unlock_irq(&mapping->i_pages);
+		return -EAGAIN;
+	}
 
 	expected_count += hpage_nr_pages(page) + page_has_private(page);
 	if (page_count(page) != expected_count ||
@@ -592,6 +596,10 @@ int migrate_huge_page_move_mapping(struct address_space *mapping,
 	xa_lock_irq(&mapping->i_pages);
 
 	pslot = radix_tree_lookup_slot(&mapping->i_pages, page_index(page));
+	if (pslot == NULL) {
+		xa_unlock_irq(&mapping->i_pages);
+		return -EAGAIN;
+	}
 
 	expected_count = 2 + page_has_private(page);
 	if (page_count(page) != expected_count ||
-- 
2.25.1

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[Greg KH]

On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
> Upstream has no this bug.

What do you mean by this?

confused,

greg k-h

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[David Hildenbrand]

On 17.02.22 08:29, Greg KH wrote:
> On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
>> Upstream has no this bug.
> 
> What do you mean by this?
> 
> confused,

Dito. If this is fixed upstream and broken in stable kernels, we'd want
either a backport of the relevant upstream fix, or if too complicated, a
stable-only fix.


-- 
Thanks,

David / dhildenb

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[李力琼]

Hi Greg,

Upstream replaces migrate_page_move_mapping() with folio_migrate_mapping(),
does not use radix tree any more. So, the upstream don't have the null
pointer bug.

We found and fix this bug on '4.19.191'.

在 2022/2/17 下午3:29, Greg KH 写道:
> On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
>> Upstream has no this bug.
> What do you mean by this?
>
> confused,
>
> greg k-h

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[李力琼]

Hi Gerg,

Sorry, i may understand your confusion.
I thought 'Upstream' as the newest code, so, I said 'Upstream has no this bug'.

Yes, we found this bug on 'Upstream v4.19.191'.

How could i submit this patch to 'longterm: 4.19'.

Thanks.

**

在 2022/2/17 下午3:59, 李力琼 写道:
> Hi Greg,
>
> Upstream replaces migrate_page_move_mapping() with 
> folio_migrate_mapping(),
> does not use radix tree any more. So, the upstream don't have the null
> pointer bug.
>
> We found and fix this bug on '4.19.191'.
>
> 在 2022/2/17 下午3:29, Greg KH 写道:
>> On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
>>> Upstream has no this bug.
>> What do you mean by this?
>>
>> confused,
>>
>> greg k-h

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[李力琼]

在 2022/2/17 下午3:51, David Hildenbrand 写道:
> On 17.02.22 08:29, Greg KH wrote:
>> On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
>>> Upstream has no this bug.
>> What do you mean by this?
>>
>> confused,
> Dito. If this is fixed upstream and broken in stable kernels, we'd want
> either a backport of the relevant upstream fix, or if too complicated, a
> stable-only fix.
>
>
There is a wrong describe， i thought 'Upstream' as the newest code.
The newest code has no this bug, i should submit this patch to "longterm:4.19".
How could i do it ?

Thanks.

Re: [PATCH] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[David Hildenbrand]

On 17.02.22 09:48, 李力琼 wrote:
> 在 2022/2/17 下午3:51, David Hildenbrand 写道:
>> On 17.02.22 08:29, Greg KH wrote:
>>> On Thu, Feb 17, 2022 at 02:38:08PM +0800, liqiong wrote:
>>>> Upstream has no this bug.
>>> What do you mean by this?
>>>
>>> confused,
>> Dito. If this is fixed upstream and broken in stable kernels, we'd want
>> either a backport of the relevant upstream fix, or if too complicated, a
>> stable-only fix.
>>
>>
> There is a wrong describe， i thought 'Upstream' as the newest code.
> The newest code has no this bug, i should submit this patch to "longterm:4.19".

See https://www.kernel.org/doc/Documentation/process/stable-kernel-rules.rst

Make sure your patch subject starts with something like "[PATCH 4.19
STABLE]" and that your patch targets that stable branch.

Make sure to describe why it doesn't apply to upstream, how it was fixed
upstream, and why we cannot simply backport the upstream way to fix it.

Thanks!

-- 
Thanks,

David / dhildenb

[PATCH] [PATCH 4.19 STABLE] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[liqiong]

Upstream doesn't use radix tree any more in migrate.c, no need this patch.

The two functions look up a slot and dereference the pointer,
If the pointer is null, the kernel would crash and dump.

The 'numad' service calls 'migrate_pages' periodically. If some slots
being replaced (Cache Eviction), the radix_tree_lookup_slot() returns
a null pointer that causes kernel crash.

"numad":  crash> bt
[exception RIP: migrate_page_move_mapping+337]

Introduce pointer checking to avoid dereference a null pointer.

Cc: <stable@vger.kernel.org> # linux-4.19.y
Signed-off-by: liqiong <liqiong@nfschina.com>
---
 mm/migrate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mm/migrate.c b/mm/migrate.c
index a69b842f95da..76f8dedc0e02 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -472,6 +472,10 @@ int migrate_page_move_mapping(struct address_space *mapping,
 
 	pslot = radix_tree_lookup_slot(&mapping->i_pages,
  					page_index(page));
+	if (pslot == NULL) {
+		xa_unlock_irq(&mapping->i_pages);
+		return -EAGAIN;
+	}
 
 	expected_count += hpage_nr_pages(page) + page_has_private(page);
 	if (page_count(page) != expected_count ||
@@ -590,6 +594,10 @@ int migrate_huge_page_move_mapping(struct address_space *mapping,
 	xa_lock_irq(&mapping->i_pages);
 
 	pslot = radix_tree_lookup_slot(&mapping->i_pages, page_index(page));
+	if (pslot == NULL) {
+		xa_unlock_irq(&mapping->i_pages);
+		return -EAGAIN;
+	}
 
 	expected_count = 2 + page_has_private(page);
 	if (page_count(page) != expected_count ||
-- 
2.25.1

Re: [PATCH] [PATCH 4.19 STABLE] mm: fix dereference a null pointer in migrate[_huge]_page_move_mapping()

[Greg KH]

On Thu, Feb 17, 2022 at 07:54:16PM +0800, liqiong wrote:
> Upstream doesn't use radix tree any more in migrate.c, no need this patch.
> 
> The two functions look up a slot and dereference the pointer,
> If the pointer is null, the kernel would crash and dump.
> 
> The 'numad' service calls 'migrate_pages' periodically. If some slots
> being replaced (Cache Eviction), the radix_tree_lookup_slot() returns
> a null pointer that causes kernel crash.
> 
> "numad":  crash> bt
> [exception RIP: migrate_page_move_mapping+337]
> 
> Introduce pointer checking to avoid dereference a null pointer.
> 
> Cc: <stable@vger.kernel.org> # linux-4.19.y
> Signed-off-by: liqiong <liqiong@nfschina.com>
> ---
>  mm/migrate.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a69b842f95da..76f8dedc0e02 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -472,6 +472,10 @@ int migrate_page_move_mapping(struct address_space *mapping,
>  
>  	pslot = radix_tree_lookup_slot(&mapping->i_pages,
>   					page_index(page));
> +	if (pslot == NULL) {
> +		xa_unlock_irq(&mapping->i_pages);
> +		return -EAGAIN;
> +	}
>  
>  	expected_count += hpage_nr_pages(page) + page_has_private(page);
>  	if (page_count(page) != expected_count ||
> @@ -590,6 +594,10 @@ int migrate_huge_page_move_mapping(struct address_space *mapping,
>  	xa_lock_irq(&mapping->i_pages);
>  
>  	pslot = radix_tree_lookup_slot(&mapping->i_pages, page_index(page));
> +	if (pslot == NULL) {
> +		xa_unlock_irq(&mapping->i_pages);
> +		return -EAGAIN;
> +	}
>  
>  	expected_count = 2 + page_has_private(page);
>  	if (page_count(page) != expected_count ||
> -- 
> 2.25.1
> 

Sorry for the delay, now queued up.

greg k-h