2022-02-17 10:38:34

by Li Qiong

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

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: <[email protected]> # v4.19-rc8
Signed-off-by: liqiong <[email protected]>
---
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

2022-02-17 11:39:01

by Li Qiong

Subject: 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

2022-02-17 12:23:28

by Li Qiong

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

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: <[email protected]> # linux-4.19.y
Signed-off-by: liqiong <[email protected]>
---
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

2022-03-17 11:20:48

by Greg Kroah-Hartman

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

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: <[email protected]> # linux-4.19.y
> Signed-off-by: liqiong <[email protected]>
> ---
> 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