[PATCH] Fix do_pages_stat to use compat_uptr_t

[PATCH] Fix do_pages_stat to use compat_uptr_t

[Christoph Berg]

For arrays with more than 16 entries, the old code would incorrectly
advance the pages pointer by 16 words instead of 16 compat_uptr_t.

Signed-off-by: Christoph Berg <myon@debian.org>
Suggested-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
---
 mm/migrate.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 8cf0f9c9599..542c81ec3ed 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2444,7 +2444,13 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
 		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
 			break;
 
-		pages += chunk_nr;
+		if (in_compat_syscall()) {
+			compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
+
+			pages32 += chunk_nr;
+			pages = (const void __user * __user *) pages32;
+		} else
+			pages += chunk_nr;
 		status += chunk_nr;
 		nr_pages -= chunk_nr;
 	}
-- 
2.47.2

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[David Hildenbrand]

On 25.06.25 17:24, Christoph Berg wrote:

Subject should start with "mm/migrate:"

> For arrays with more than 16 entries, the old code would incorrectly
> advance the pages pointer by 16 words instead of 16 compat_uptr_t.
> 
> Signed-off-by: Christoph Berg <myon@debian.org>
> Suggested-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

Likely we want a

Fixes:

and then this is probably "Reported-by:" paired with a "Closes:" link
to any such report.

But I'm wondering how long this has already been like that. :)

> ---
>   mm/migrate.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 8cf0f9c9599..542c81ec3ed 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2444,7 +2444,13 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>   		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
>   			break;
>   
> -		pages += chunk_nr;
> +		if (in_compat_syscall()) {
> +			compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
> +
> +			pages32 += chunk_nr;
> +			pages = (const void __user * __user *) pages32;
> +		} else
> +			pages += chunk_nr;
>   		status += chunk_nr;
>   		nr_pages -= chunk_nr;
>   	}

Something a bit more elegant might be:

diff --git a/mm/migrate.c b/mm/migrate.c
index ea8c74d996592..fc99448771041 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2398,6 +2398,7 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages,
  
  static int get_compat_pages_array(const void __user *chunk_pages[],
                                   const void __user * __user *pages,
+                                 unsigned long chunk_offs,
                                   unsigned long chunk_nr)
  {
         compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
@@ -2405,7 +2406,7 @@ static int get_compat_pages_array(const void __user *chunk_pages[],
         int i;
  
         for (i = 0; i < chunk_nr; i++) {
-               if (get_user(p, pages32 + i))
+               if (get_user(p, pages32 + i + chunk_offs))
                         return -EFAULT;
                 chunk_pages[i] = compat_ptr(p);
         }
@@ -2424,13 +2425,14 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
  #define DO_PAGES_STAT_CHUNK_NR 16UL
         const void __user *chunk_pages[DO_PAGES_STAT_CHUNK_NR];
         int chunk_status[DO_PAGES_STAT_CHUNK_NR];
+       unsigned long chunk_offs = 0;
  
         while (nr_pages) {
                 unsigned long chunk_nr = min(nr_pages, DO_PAGES_STAT_CHUNK_NR);
  
                 if (in_compat_syscall()) {
                         if (get_compat_pages_array(chunk_pages, pages,
-                                                  chunk_nr))
+                                                  chunk_offs, chunk_nr))
                                 break;
                 } else {
                         if (copy_from_user(chunk_pages, pages,
@@ -2440,11 +2442,11 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
  
                 do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
  
-               if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
+               if (copy_to_user(status + chunk_offs, chunk_status,
+                                chunk_nr * sizeof(*status)))
                         break;
  
-               pages += chunk_nr;
-               status += chunk_nr;
+               chunk_offs += chunk_nr;
                 nr_pages -= chunk_nr;
         }
         return nr_pages ? -EFAULT : 0;

(untested, of course)


-- 
Cheers,

David / dhildenb

[PATCH v2] mm/migrate: Fix do_pages_stat in 32-bit mode

[Christoph Berg]

Re: David Hildenbrand
> Subject should start with "mm/migrate:"
> Likely we want a
> Fixes:
> and then this is probably "Reported-by:" paired with a "Closes:" link
> to any such report.

I included these now, except for "Closes:" which I have to idea what
to put in.

> But I'm wondering how long this has already been like that. :)

The now-offending "pages += chunk_nr" line is from 2010, but I think
the bug is rather from 5b1b561ba73c8ab9c98e5dfd14dc7ee47efb6530 (2021)
which reshuffled the array-vs-32-bit handling.

> Something a bit more elegant might be:

Thanks, I used your patch draft with some minor changes.

>  static int get_compat_pages_array(const void __user *chunk_pages[],
>                                   const void __user * __user *pages,
> +                                 unsigned long chunk_offs,

I replaced chunk_offs with "chunk_offset" since "offs" looked too much
like plural (list of offsets) to me.

>                 if (in_compat_syscall()) {
>                         if (get_compat_pages_array(chunk_pages, pages,
> -                                                  chunk_nr))
> +                                                  chunk_offs, chunk_nr))
>                                 break;
>                 } else {
>                         if (copy_from_user(chunk_pages, pages,

The else branch here needs tweaking as well:

                } else {
-                       if (copy_from_user(chunk_pages, pages,
+                       if (copy_from_user(chunk_pages, pages + chunk_offset,
                                      chunk_nr * sizeof(*chunk_pages)))


> @@ -2440,11 +2442,11 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>                 do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
> -               if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
> +               if (copy_to_user(status + chunk_offs, chunk_status,
> +                                chunk_nr * sizeof(*status)))

This seems to work, but honestly I am wondering, if copy_from_user
needs a special 32-bit case, doesn't copy_to_user need special casing
as well?

> (untested, of course)

The attached patch makes PG18's new numa test pass on amd64 kernels
both in amd64 and i386 userlands.

(In the meantime, PG git head got a workaround that limits the chunk
size to the same 16 as used in do_pages_stat; I tested with the
version before that.)

Christoph


From fdbcbc88825bc2e857dfeeebc91d62864e0774dd Mon Sep 17 00:00:00 2001
From: Christoph Berg <myon@debian.org>
Date: Tue, 24 Jun 2025 16:44:27 +0200
Subject: [PATCH v2] mm/migrate: Fix do_pages_stat in 32-bit mode

For arrays with more than 16 entries, the old code would incorrectly
advance the pages pointer by 16 words instead of 16 compat_uptr_t.
Fix by doing the pointer arithmetic inside get_compat_pages_array where
pages32 is already a correctly-typed pointer.

Discovered while working on PostgreSQL 18's new NUMA introspection code.

Signed-off-by: Christoph Berg <myon@debian.org>
Reported-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reported-by: Tomas Vondra <tomas@vondra.me>
Suggested-by: David Hildenbrand <david@redhat.com>
Fixes: 5b1b561ba73c8ab9c98e5dfd14dc7ee47efb6530
---
 mm/migrate.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 8cf0f9c9599d..2c88f3b33833 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2399,6 +2399,7 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages,
 
 static int get_compat_pages_array(const void __user *chunk_pages[],
 				  const void __user * __user *pages,
+				  unsigned long chunk_offset,
 				  unsigned long chunk_nr)
 {
 	compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
@@ -2406,7 +2407,7 @@ static int get_compat_pages_array(const void __user *chunk_pages[],
 	int i;
 
 	for (i = 0; i < chunk_nr; i++) {
-		if (get_user(p, pages32 + i))
+		if (get_user(p, pages32 + chunk_offset + i))
 			return -EFAULT;
 		chunk_pages[i] = compat_ptr(p);
 	}
@@ -2425,27 +2426,28 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
 #define DO_PAGES_STAT_CHUNK_NR 16UL
 	const void __user *chunk_pages[DO_PAGES_STAT_CHUNK_NR];
 	int chunk_status[DO_PAGES_STAT_CHUNK_NR];
+	unsigned long chunk_offset = 0;
 
 	while (nr_pages) {
 		unsigned long chunk_nr = min(nr_pages, DO_PAGES_STAT_CHUNK_NR);
 
 		if (in_compat_syscall()) {
 			if (get_compat_pages_array(chunk_pages, pages,
-						   chunk_nr))
+						   chunk_offset, chunk_nr))
 				break;
 		} else {
-			if (copy_from_user(chunk_pages, pages,
+			if (copy_from_user(chunk_pages, pages + chunk_offset,
 				      chunk_nr * sizeof(*chunk_pages)))
 				break;
 		}
 
 		do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
 
-		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
+		if (copy_to_user(status + chunk_offset, chunk_status,
+				 chunk_nr * sizeof(*status)))
 			break;
 
-		pages += chunk_nr;
-		status += chunk_nr;
+		chunk_offset += chunk_nr;
 		nr_pages -= chunk_nr;
 	}
 	return nr_pages ? -EFAULT : 0;
-- 
2.47.2

Re: [PATCH v2] mm/migrate: Fix do_pages_stat in 32-bit mode

[Zi Yan]

On 1 Jul 2025, at 12:58, Christoph Berg wrote:

> Re: David Hildenbrand
>> Subject should start with "mm/migrate:"
>> Likely we want a
>> Fixes:
>> and then this is probably "Reported-by:" paired with a "Closes:" link
>> to any such report.
>
> I included these now, except for "Closes:" which I have to idea what
> to put in.

Fixes should be:

Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")

Closes could be a link to the bug report.

>
>> But I'm wondering how long this has already been like that. :)
>
> The now-offending "pages += chunk_nr" line is from 2010, but I think
> the bug is rather from 5b1b561ba73c8ab9c98e5dfd14dc7ee47efb6530 (2021)
> which reshuffled the array-vs-32-bit handling.
>
>> Something a bit more elegant might be:
>
> Thanks, I used your patch draft with some minor changes.
>
>>  static int get_compat_pages_array(const void __user *chunk_pages[],
>>                                   const void __user * __user *pages,
>> +                                 unsigned long chunk_offs,
>
> I replaced chunk_offs with "chunk_offset" since "offs" looked too much
> like plural (list of offsets) to me.
>
>>                 if (in_compat_syscall()) {
>>                         if (get_compat_pages_array(chunk_pages, pages,
>> -                                                  chunk_nr))
>> +                                                  chunk_offs, chunk_nr))
>>                                 break;
>>                 } else {
>>                         if (copy_from_user(chunk_pages, pages,
>
> The else branch here needs tweaking as well:
>
>                 } else {
> -                       if (copy_from_user(chunk_pages, pages,
> +                       if (copy_from_user(chunk_pages, pages + chunk_offset,
>                                       chunk_nr * sizeof(*chunk_pages)))
>
>
>> @@ -2440,11 +2442,11 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>>                 do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
>> -               if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
>> +               if (copy_to_user(status + chunk_offs, chunk_status,
>> +                                chunk_nr * sizeof(*status)))
>
> This seems to work, but honestly I am wondering, if copy_from_user
> needs a special 32-bit case, doesn't copy_to_user need special casing
> as well?
>
>> (untested, of course)
>
> The attached patch makes PG18's new numa test pass on amd64 kernels
> both in amd64 and i386 userlands.
>
> (In the meantime, PG git head got a workaround that limits the chunk
> size to the same 16 as used in do_pages_stat; I tested with the
> version before that.)
>
> Christoph
>
>
> From fdbcbc88825bc2e857dfeeebc91d62864e0774dd Mon Sep 17 00:00:00 2001
> From: Christoph Berg <myon@debian.org>
> Date: Tue, 24 Jun 2025 16:44:27 +0200
> Subject: [PATCH v2] mm/migrate: Fix do_pages_stat in 32-bit mode
>
> For arrays with more than 16 entries, the old code would incorrectly
> advance the pages pointer by 16 words instead of 16 compat_uptr_t.
> Fix by doing the pointer arithmetic inside get_compat_pages_array where
> pages32 is already a correctly-typed pointer.
>
> Discovered while working on PostgreSQL 18's new NUMA introspection code.
>
> Signed-off-by: Christoph Berg <myon@debian.org>
> Reported-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
> Reported-by: Tomas Vondra <tomas@vondra.me>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 5b1b561ba73c8ab9c98e5dfd14dc7ee47efb6530
> ---
>  mm/migrate.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 8cf0f9c9599d..2c88f3b33833 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2399,6 +2399,7 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages,
>
>  static int get_compat_pages_array(const void __user *chunk_pages[],
>  				  const void __user * __user *pages,
> +				  unsigned long chunk_offset,
>  				  unsigned long chunk_nr)
>  {
>  	compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
> @@ -2406,7 +2407,7 @@ static int get_compat_pages_array(const void __user *chunk_pages[],
>  	int i;
>
>  	for (i = 0; i < chunk_nr; i++) {
> -		if (get_user(p, pages32 + i))
> +		if (get_user(p, pages32 + chunk_offset + i))
>  			return -EFAULT;
>  		chunk_pages[i] = compat_ptr(p);
>  	}
> @@ -2425,27 +2426,28 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>  #define DO_PAGES_STAT_CHUNK_NR 16UL
>  	const void __user *chunk_pages[DO_PAGES_STAT_CHUNK_NR];
>  	int chunk_status[DO_PAGES_STAT_CHUNK_NR];
> +	unsigned long chunk_offset = 0;
>
>  	while (nr_pages) {
>  		unsigned long chunk_nr = min(nr_pages, DO_PAGES_STAT_CHUNK_NR);
>
>  		if (in_compat_syscall()) {
>  			if (get_compat_pages_array(chunk_pages, pages,
> -						   chunk_nr))
> +						   chunk_offset, chunk_nr))
>  				break;
>  		} else {
> -			if (copy_from_user(chunk_pages, pages,
> +			if (copy_from_user(chunk_pages, pages + chunk_offset,
>  				      chunk_nr * sizeof(*chunk_pages)))
>  				break;
>  		}
>
>  		do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
>
> -		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
> +		if (copy_to_user(status + chunk_offset, chunk_status,
> +				 chunk_nr * sizeof(*status)))
>  			break;
>
> -		pages += chunk_nr;
> -		status += chunk_nr;
> +		chunk_offset += chunk_nr;
>  		nr_pages -= chunk_nr;
>  	}
>  	return nr_pages ? -EFAULT : 0;
> -- 
> 2.47.2


Best Regards,
Yan, Zi

Re: [PATCH v3] mm/migrate: Fix do_pages_stat in 32-bit mode

[Christoph Berg]

Re: Zi Yan
> Fixes should be:
> 
> Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")
> 
> Closes could be a link to the bug report.

Updated, thanks.

> > This seems to work, but honestly I am wondering, if copy_from_user
> > needs a special 32-bit case, doesn't copy_to_user need special casing
> > as well?

Scratch that, it works because an int[] is copied back, and that's
always the same size.

So I think the patch is good to go.

Christoph


From 426c93d558572248273cf386ca784626ae431413 Mon Sep 17 00:00:00 2001
From: Christoph Berg <myon@debian.org>
Date: Tue, 24 Jun 2025 16:44:27 +0200
Subject: [PATCH v3] mm/migrate: Fix do_pages_stat in 32-bit mode

For arrays with more than 16 entries, the old code would incorrectly
advance the pages pointer by 16 words instead of 16 compat_uptr_t.
Fix by doing the pointer arithmetic inside get_compat_pages_array where
pages32 is already a correctly-typed pointer.

Discovered while working on PostgreSQL 18's new NUMA introspection code.

Signed-off-by: Christoph Berg <myon@debian.org>
Suggested-by: David Hildenbrand <david@redhat.com>
Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")
Reported-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reported-by: Tomas Vondra <tomas@vondra.me>
Closes: https://www.postgresql.org/message-id/flat/6342f601-77de-4ee0-8c2a-3deb50ceac5b%40vondra.me#86402e3d80c031788f5f55b42c459471
---
 mm/migrate.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 8cf0f9c9599d..2c88f3b33833 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2399,6 +2399,7 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages,
 
 static int get_compat_pages_array(const void __user *chunk_pages[],
 				  const void __user * __user *pages,
+				  unsigned long chunk_offset,
 				  unsigned long chunk_nr)
 {
 	compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
@@ -2406,7 +2407,7 @@ static int get_compat_pages_array(const void __user *chunk_pages[],
 	int i;
 
 	for (i = 0; i < chunk_nr; i++) {
-		if (get_user(p, pages32 + i))
+		if (get_user(p, pages32 + chunk_offset + i))
 			return -EFAULT;
 		chunk_pages[i] = compat_ptr(p);
 	}
@@ -2425,27 +2426,28 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
 #define DO_PAGES_STAT_CHUNK_NR 16UL
 	const void __user *chunk_pages[DO_PAGES_STAT_CHUNK_NR];
 	int chunk_status[DO_PAGES_STAT_CHUNK_NR];
+	unsigned long chunk_offset = 0;
 
 	while (nr_pages) {
 		unsigned long chunk_nr = min(nr_pages, DO_PAGES_STAT_CHUNK_NR);
 
 		if (in_compat_syscall()) {
 			if (get_compat_pages_array(chunk_pages, pages,
-						   chunk_nr))
+						   chunk_offset, chunk_nr))
 				break;
 		} else {
-			if (copy_from_user(chunk_pages, pages,
+			if (copy_from_user(chunk_pages, pages + chunk_offset,
 				      chunk_nr * sizeof(*chunk_pages)))
 				break;
 		}
 
 		do_pages_stat_array(mm, chunk_nr, chunk_pages, chunk_status);
 
-		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
+		if (copy_to_user(status + chunk_offset, chunk_status,
+				 chunk_nr * sizeof(*status)))
 			break;
 
-		pages += chunk_nr;
-		status += chunk_nr;
+		chunk_offset += chunk_nr;
 		nr_pages -= chunk_nr;
 	}
 	return nr_pages ? -EFAULT : 0;
-- 
2.47.2

Re: [PATCH v3] mm/migrate: Fix do_pages_stat in 32-bit mode

[David Hildenbrand]

On 01.07.25 19:55, Christoph Berg wrote:
> Re: Zi Yan
>> Fixes should be:
>>
>> Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")
>>
>> Closes could be a link to the bug report.
> 
> Updated, thanks.
> 
>>> This seems to work, but honestly I am wondering, if copy_from_user
>>> needs a special 32-bit case, doesn't copy_to_user need special casing
>>> as well?
> 
> Scratch that, it works because an int[] is copied back, and that's
> always the same size.
> 
> So I think the patch is good to go.
> 
> Christoph
> 
> 

If you have to resend, next time resend the full patch separately, not 
as reply to the previous version.

>  From 426c93d558572248273cf386ca784626ae431413 Mon Sep 17 00:00:00 2001
> From: Christoph Berg <myon@debian.org>
> Date: Tue, 24 Jun 2025 16:44:27 +0200
> Subject: [PATCH v3] mm/migrate: Fix do_pages_stat in 32-bit mode

s/32-bit mode/compat mode/ ?

Because on native 32bit it should be working fine.

> 
> For arrays with more than 16 entries, the old code would incorrectly
> advance the pages pointer by 16 words instead of 16 compat_uptr_t.
> Fix by doing the pointer arithmetic inside get_compat_pages_array where
> pages32 is already a correctly-typed pointer.
> 
> Discovered while working on PostgreSQL 18's new NUMA introspection code.
> 
> Signed-off-by: Christoph Berg <myon@debian.org>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")

Hmm, still not sure if 5b1b561ba73c really introduced the issue. I think 
it only messed with the "pages" pointer, not with the "status" pointer?

Hmmmm


I assume we want to Cc stable. @Andrew can do that.

> Reported-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
> Reported-by: Tomas Vondra <tomas@vondra.me>
> Closes: https://www.postgresql.org/message-id/flat/6342f601-77de-4ee0-8c2a-3deb50ceac5b%40vondra.me#86402e3d80c031788f5f55b42c459471
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v3] mm/migrate: Fix do_pages_stat in 32-bit mode

[Christoph Berg]

Re: David Hildenbrand
> > For arrays with more than 16 entries, the old code would incorrectly
> > advance the pages pointer by 16 words instead of 16 compat_uptr_t.
> > Fix by doing the pointer arithmetic inside get_compat_pages_array where
> > pages32 is already a correctly-typed pointer.
> > 
> > Discovered while working on PostgreSQL 18's new NUMA introspection code.
> > 
> > Signed-off-by: Christoph Berg <myon@debian.org>
> > Suggested-by: David Hildenbrand <david@redhat.com>
> > Fixes: 5b1b561ba73c ("mm: simplify compat_sys_move_pages")
> 
> Hmm, still not sure if 5b1b561ba73c really introduced the issue. I think it
> only messed with the "pages" pointer, not with the "status" pointer?

"pages" was the broken one. "status" isn't affected by compat mode.

> Acked-by: David Hildenbrand <david@redhat.com>

Thanks!

Christoph

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[Andrew Morton]

On Wed, 25 Jun 2025 17:24:14 +0200 Christoph Berg <myon@debian.org> wrote:

> For arrays with more than 16 entries, the old code would incorrectly
> advance the pages pointer by 16 words instead of 16 compat_uptr_t.
> 
> ...
>
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2444,7 +2444,13 @@ static int do_pages_stat(struct mm_struct *mm, unsigned long nr_pages,
>  		if (copy_to_user(status, chunk_status, chunk_nr * sizeof(*status)))
>  			break;
>  
> -		pages += chunk_nr;
> +		if (in_compat_syscall()) {
> +			compat_uptr_t __user *pages32 = (compat_uptr_t __user *)pages;
> +
> +			pages32 += chunk_nr;
> +			pages = (const void __user * __user *) pages32;
> +		} else
> +			pages += chunk_nr;
>  		status += chunk_nr;
>  		nr_pages -= chunk_nr;
>  	}

Seems this has been present since 2010.

I'll update the Subject: as David suggests and I'll add a cc:stable,
thanks.  I'll also add a note that David suggested an alternative, so
please let's advance that option.

Also, a coding-style tweak:

--- a/mm/migrate.c~fix-do_pages_stat-to-use-compat_uptr_t-fix
+++ a/mm/migrate.c
@@ -2449,8 +2449,9 @@ static int do_pages_stat(struct mm_struc
 
 			pages32 += chunk_nr;
 			pages = (const void __user * __user *) pages32;
-		} else
+		} else {
 			pages += chunk_nr;
+		}
 		status += chunk_nr;
 		nr_pages -= chunk_nr;
 	}
_

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[Christoph Berg]

Re: Andrew Morton
> I'll update the Subject: as David suggests and I'll add a cc:stable,
> thanks.  I'll also add a note that David suggested an alternative, so
> please let's advance that option.

Sorry, I'm new here. Do I have to do anything now? The above sounds
like the alternative coding by David would be preferred, but the other
mails say the patches have already been pushed to a hotfix branch? Or
are we doing both, i.e. stable gets the simple fix and next the pretty
one?

Christoph

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[Andrew Morton]

On Wed, 25 Jun 2025 23:10:13 +0200 Christoph Berg <myon@debian.org> wrote:

> Re: Andrew Morton
> > I'll update the Subject: as David suggests and I'll add a cc:stable,
> > thanks.  I'll also add a note that David suggested an alternative, so
> > please let's advance that option.
> 
> Sorry, I'm new here. Do I have to do anything now? The above sounds
> like the alternative coding by David would be preferred, but the other
> mails say the patches have already been pushed to a hotfix branch? Or
> are we doing both, i.e. stable gets the simple fix and next the pretty
> one?

Well I added the v1 patch for testing and so it doesn't get lost -
fixing the bug is the most important thing.

But if a nicer v2 patch is sent in the next few days, I can switch to
that version.

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[David Hildenbrand]

On 25.06.25 23:15, Andrew Morton wrote:
> On Wed, 25 Jun 2025 23:10:13 +0200 Christoph Berg <myon@debian.org> wrote:
> 
>> Re: Andrew Morton
>>> I'll update the Subject: as David suggests and I'll add a cc:stable,
>>> thanks.  I'll also add a note that David suggested an alternative, so
>>> please let's advance that option.
>>
>> Sorry, I'm new here. Do I have to do anything now? The above sounds
>> like the alternative coding by David would be preferred

See if my proposal makes sense, and then incorporate+test it.

You can feel free to add my

Suggested-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] Fix do_pages_stat to use compat_uptr_t

[Christoph Berg]

Re: David Hildenbrand
> See if my proposal makes sense, and then incorporate+test it.
> 
> You can feel free to add my
> 
> Suggested-by: David Hildenbrand <david@redhat.com>

Thanks, will do. But it will be next week, currently busy with the ham
radio fair in Friedrichshafen.

Christoph