1. Patchew
  2. linux
  3. View series

[PATCH] mm/mremap: fix regression in vrm->new_addr check

Carlos Llamas posted 1 patch 1 month ago
There is a newer version of this series
mm/mremap.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
[PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
checks") moved the sanity check for vrm->new_addr from mremap_to() to
check_mremap_params().

However, this caused a regression as vrm->new_addr is now checked even
when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
case, vrm->new_addr can be garbage and create unexpected failures.

Fix this by moving the new_addr check after the vrm_implies_new_addr()
guard. This ensures that the new_addr is only checked when the user has
specified one explicitly.

Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 mm/mremap.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index e618a706aff5..692acb0f9ea2 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1771,18 +1771,17 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
 	 * for DOS-emu "duplicate shm area" thing. But
 	 * a zero new-len is nonsensical.
 	 */
-	if (!vrm->new_len)
-		return -EINVAL;
-
-	/* Is the new length or address silly? */
-	if (vrm->new_len > TASK_SIZE ||
-	    vrm->new_addr > TASK_SIZE - vrm->new_len)
+	if (!vrm->new_len || vrm->new_len > TASK_SIZE)
 		return -EINVAL;
 
 	/* Remainder of checks are for cases with specific new_addr. */
 	if (!vrm_implies_new_addr(vrm))
 		return 0;
 
+	/* Is the new address silly? */
+	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
+		return -EINVAL;
+
 	/* The new address must be page-aligned. */
 	if (offset_in_page(vrm->new_addr))
 		return -EINVAL;
-- 
2.51.0.268.g9569e192d0-goog
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Lorenzo Stoakes 1 month ago 
On Thu, Aug 28, 2025 at 03:26:52AM +0000, Carlos Llamas wrote:
> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> check_mremap_params().
>
> However, this caused a regression as vrm->new_addr is now checked even
> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> case, vrm->new_addr can be garbage and create unexpected failures.

Yikes, sorry my mistake.

>
> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> guard. This ensures that the new_addr is only checked when the user has
> specified one explicitly.
>
> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

You need a Cc: Stable.

> ---
>  mm/mremap.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e618a706aff5..692acb0f9ea2 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1771,18 +1771,17 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
>  	 * for DOS-emu "duplicate shm area" thing. But
>  	 * a zero new-len is nonsensical.
>  	 */
> -	if (!vrm->new_len)
> -		return -EINVAL;

Please don't refactor code at the same time, especially as this is a hotfix.

This line is associated with the above comment.

> -
> -	/* Is the new length or address silly? */
> -	if (vrm->new_len > TASK_SIZE ||
> -	    vrm->new_addr > TASK_SIZE - vrm->new_len)
> +	if (!vrm->new_len || vrm->new_len > TASK_SIZE)
>  		return -EINVAL;

Yeah, it's cute but by removing the 'silly' comment you're making this
associated with the comment above and... yeah.

This should be:

	/*
	 * We allow a zero old-len as a special case
	 * for DOS-emu "duplicate shm area" thing. But
	 * a zero new-len is nonsensical.
	 */
	if (!vrm->new_len)
		return -EINVAL;

	/* Is the new length silly? */
	if (vrm->new_len > TASK_SIZE)
		return -EINVAL;

>
>  	/* Remainder of checks are for cases with specific new_addr. */
>  	if (!vrm_implies_new_addr(vrm))
>  		return 0;
>
> +	/* Is the new address silly? */
> +	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
> +		return -EINVAL;
> +

Obviously this bit is fine :)

Sorry again, this was just a mistake on my part.

>  	/* The new address must be page-aligned. */
>  	if (offset_in_page(vrm->new_addr))
>  		return -EINVAL;
> --
> 2.51.0.268.g9569e192d0-goog
>

I'm curious why only you guys have seen it, a theory is bionic is sending random
stuff to this parameter when unspecified, and glibc is not.

But obviously this fix is correct, and the original code needs fixing.

Please respin a v2 as per above.

Cheers, Lorenzo
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Vlastimil Babka 1 month ago 
On 8/28/25 07:38, Lorenzo Stoakes wrote:
> On Thu, Aug 28, 2025 at 03:26:52AM +0000, Carlos Llamas wrote:
>> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
>> checks") moved the sanity check for vrm->new_addr from mremap_to() to
>> check_mremap_params().
>>
>> However, this caused a regression as vrm->new_addr is now checked even
>> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
>> case, vrm->new_addr can be garbage and create unexpected failures.
> 
> Yikes, sorry my mistake.
> 
>>
>> Fix this by moving the new_addr check after the vrm_implies_new_addr()
>> guard. This ensures that the new_addr is only checked when the user has
>> specified one explicitly.
>>
>> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
>> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> 
> You need a Cc: Stable.

No need as the commit being fixed is from 6.17-rc1?
But it's better to use "[PATCH mm-hotfixes]" to make sure it goes to 6.17
and not the next merge window.
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Lorenzo Stoakes 1 month ago 
On Thu, Aug 28, 2025 at 04:21:05PM +0200, Vlastimil Babka wrote:
> On 8/28/25 07:38, Lorenzo Stoakes wrote:
> > On Thu, Aug 28, 2025 at 03:26:52AM +0000, Carlos Llamas wrote:
> >> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> >> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> >> check_mremap_params().
> >>
> >> However, this caused a regression as vrm->new_addr is now checked even
> >> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> >> case, vrm->new_addr can be garbage and create unexpected failures.
> >
> > Yikes, sorry my mistake.
> >
> >>
> >> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> >> guard. This ensures that the new_addr is only checked when the user has
> >> specified one explicitly.
> >>
> >> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> >> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> >
> > You need a Cc: Stable.
>
> No need as the commit being fixed is from 6.17-rc1?
> But it's better to use "[PATCH mm-hotfixes]" to make sure it goes to 6.17
> and not the next merge window.
>

Ah haha really? I'm losing track of my patches.

Yeah sure as per Vlasta then Carlos :)
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
On Thu, Aug 28, 2025 at 03:22:54PM +0100, Lorenzo Stoakes wrote:
> On Thu, Aug 28, 2025 at 04:21:05PM +0200, Vlastimil Babka wrote:
> > On 8/28/25 07:38, Lorenzo Stoakes wrote:
> > > On Thu, Aug 28, 2025 at 03:26:52AM +0000, Carlos Llamas wrote:
> > >> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> > >> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> > >> check_mremap_params().
> > >>
> > >> However, this caused a regression as vrm->new_addr is now checked even
> > >> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> > >> case, vrm->new_addr can be garbage and create unexpected failures.
> > >
> > > Yikes, sorry my mistake.
> > >
> > >>
> > >> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> > >> guard. This ensures that the new_addr is only checked when the user has
> > >> specified one explicitly.
> > >>
> > >> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> > >> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > >
> > > You need a Cc: Stable.
> >
> > No need as the commit being fixed is from 6.17-rc1?
> > But it's better to use "[PATCH mm-hotfixes]" to make sure it goes to 6.17
> > and not the next merge window.
> >
> 
> Ah haha really? I'm losing track of my patches.
> 
> Yeah sure as per Vlasta then Carlos :)

Oops, sorry my v2 raced with this.
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Vlastimil Babka 1 month ago 
On 8/28/25 16:29, Carlos Llamas wrote:
> On Thu, Aug 28, 2025 at 03:22:54PM +0100, Lorenzo Stoakes wrote:
>> On Thu, Aug 28, 2025 at 04:21:05PM +0200, Vlastimil Babka wrote:
>> > On 8/28/25 07:38, Lorenzo Stoakes wrote:
>> > > On Thu, Aug 28, 2025 at 03:26:52AM +0000, Carlos Llamas wrote:
>> > >> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
>> > >> checks") moved the sanity check for vrm->new_addr from mremap_to() to
>> > >> check_mremap_params().
>> > >>
>> > >> However, this caused a regression as vrm->new_addr is now checked even
>> > >> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
>> > >> case, vrm->new_addr can be garbage and create unexpected failures.
>> > >
>> > > Yikes, sorry my mistake.
>> > >
>> > >>
>> > >> Fix this by moving the new_addr check after the vrm_implies_new_addr()
>> > >> guard. This ensures that the new_addr is only checked when the user has
>> > >> specified one explicitly.
>> > >>
>> > >> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
>> > >> Signed-off-by: Carlos Llamas <cmllamas@google.com>
>> > >
>> > > You need a Cc: Stable.
>> >
>> > No need as the commit being fixed is from 6.17-rc1?
>> > But it's better to use "[PATCH mm-hotfixes]" to make sure it goes to 6.17
>> > and not the next merge window.
>> >
>> 
>> Ah haha really? I'm losing track of my patches.
>> 
>> Yeah sure as per Vlasta then Carlos :)
> 
> Oops, sorry my v2 raced with this.

Nevermind, Andrew can fixup locally. Thanks :)
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
On Thu, Aug 28, 2025 at 06:38:09AM +0100, Lorenzo Stoakes wrote:
> I'm curious why only you guys have seen it, a theory is bionic is sending random
> stuff to this parameter when unspecified, and glibc is not.

I honestly don't know and it could very well be only with bionic or
clang. I'll try to find out more about this, maybe glibc implementation
zeros out the 'new_addres' register and bionic doesn't.

> 
> But obviously this fix is correct, and the original code needs fixing.
> 
> Please respin a v2 as per above.

Sounds good.
[PATCH v2] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
checks") moved the sanity check for vrm->new_addr from mremap_to() to
check_mremap_params().

However, this caused a regression as vrm->new_addr is now checked even
when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
case, vrm->new_addr can be garbage and create unexpected failures.

Fix this by moving the new_addr check after the vrm_implies_new_addr()
guard. This ensures that the new_addr is only checked when the user has
specified one explicitly.

Cc: stable@vger.kernel.org
Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
v2:
 - split out vrm->new_len into individual checks
 - cc stable, collect tags

v1:
https://lore.kernel.org/all/20250828032653.521314-1-cmllamas@google.com/

 mm/mremap.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/mm/mremap.c b/mm/mremap.c
index e618a706aff5..35de0a7b910e 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -1774,15 +1774,18 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
 	if (!vrm->new_len)
 		return -EINVAL;
 
-	/* Is the new length or address silly? */
-	if (vrm->new_len > TASK_SIZE ||
-	    vrm->new_addr > TASK_SIZE - vrm->new_len)
+	/* Is the new length silly? */
+	if (vrm->new_len > TASK_SIZE)
 		return -EINVAL;
 
 	/* Remainder of checks are for cases with specific new_addr. */
 	if (!vrm_implies_new_addr(vrm))
 		return 0;
 
+	/* Is the new address silly? */
+	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
+		return -EINVAL;
+
 	/* The new address must be page-aligned. */
 	if (offset_in_page(vrm->new_addr))
 		return -EINVAL;
-- 
2.51.0.268.g9569e192d0-goog
Re: [PATCH v2] mm/mremap: fix regression in vrm->new_addr check
Posted by Lorenzo Stoakes 1 month ago 
(For future reference) please send separately rather than in reply to first
:)

Otherwise harder for me to find your series!

On Thu, Aug 28, 2025 at 02:26:56PM +0000, Carlos Llamas wrote:
> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> check_mremap_params().
>
> However, this caused a regression as vrm->new_addr is now checked even
> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> case, vrm->new_addr can be garbage and create unexpected failures.
>
> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> guard. This ensures that the new_addr is only checked when the user has
> specified one explicitly.
>
> Cc: stable@vger.kernel.org

Yeah oopsies on me suggesting this :P losing track of my own patches :)

> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

And again thanks so much for this! :)

> ---
> v2:
>  - split out vrm->new_len into individual checks
>  - cc stable, collect tags
>
> v1:
> https://lore.kernel.org/all/20250828032653.521314-1-cmllamas@google.com/
>
>  mm/mremap.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e618a706aff5..35de0a7b910e 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1774,15 +1774,18 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
>  	if (!vrm->new_len)
>  		return -EINVAL;
>
> -	/* Is the new length or address silly? */
> -	if (vrm->new_len > TASK_SIZE ||
> -	    vrm->new_addr > TASK_SIZE - vrm->new_len)
> +	/* Is the new length silly? */
> +	if (vrm->new_len > TASK_SIZE)
>  		return -EINVAL;
>
>  	/* Remainder of checks are for cases with specific new_addr. */
>  	if (!vrm_implies_new_addr(vrm))
>  		return 0;
>
> +	/* Is the new address silly? */
> +	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
> +		return -EINVAL;
> +
>  	/* The new address must be page-aligned. */
>  	if (offset_in_page(vrm->new_addr))
>  		return -EINVAL;
> --
> 2.51.0.268.g9569e192d0-goog
>

Cheers, Lorenzo
Re: [PATCH v2] mm/mremap: fix regression in vrm->new_addr check
Posted by Vlastimil Babka 1 month ago 
On 8/28/25 16:26, Carlos Llamas wrote:
> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> check_mremap_params().
> 
> However, this caused a regression as vrm->new_addr is now checked even
> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> case, vrm->new_addr can be garbage and create unexpected failures.
> 
> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> guard. This ensures that the new_addr is only checked when the user has
> specified one explicitly.
> 
> Cc: stable@vger.kernel.org

Not necessary, but for mm-hotfixes please, Andrew.

> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

> ---
> v2:
>  - split out vrm->new_len into individual checks
>  - cc stable, collect tags
> 
> v1:
> https://lore.kernel.org/all/20250828032653.521314-1-cmllamas@google.com/
> 
>  mm/mremap.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e618a706aff5..35de0a7b910e 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1774,15 +1774,18 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
>  	if (!vrm->new_len)
>  		return -EINVAL;
>  
> -	/* Is the new length or address silly? */
> -	if (vrm->new_len > TASK_SIZE ||
> -	    vrm->new_addr > TASK_SIZE - vrm->new_len)
> +	/* Is the new length silly? */
> +	if (vrm->new_len > TASK_SIZE)
>  		return -EINVAL;
>  
>  	/* Remainder of checks are for cases with specific new_addr. */
>  	if (!vrm_implies_new_addr(vrm))
>  		return 0;
>  
> +	/* Is the new address silly? */
> +	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
> +		return -EINVAL;
> +
>  	/* The new address must be page-aligned. */
>  	if (offset_in_page(vrm->new_addr))
>  		return -EINVAL;
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Liam R. Howlett 1 month ago 
* Carlos Llamas <cmllamas@google.com> [250827 23:27]:
> Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> checks") moved the sanity check for vrm->new_addr from mremap_to() to
> check_mremap_params().
> 
> However, this caused a regression as vrm->new_addr is now checked even
> when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> case, vrm->new_addr can be garbage and create unexpected failures.
> 
> Fix this by moving the new_addr check after the vrm_implies_new_addr()
> guard. This ensures that the new_addr is only checked when the user has
> specified one explicitly.
> 
> Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

I assume this showed up with clang?

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/mremap.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/mremap.c b/mm/mremap.c
> index e618a706aff5..692acb0f9ea2 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -1771,18 +1771,17 @@ static unsigned long check_mremap_params(struct vma_remap_struct *vrm)
>  	 * for DOS-emu "duplicate shm area" thing. But
>  	 * a zero new-len is nonsensical.
>  	 */
> -	if (!vrm->new_len)
> -		return -EINVAL;
> -
> -	/* Is the new length or address silly? */
> -	if (vrm->new_len > TASK_SIZE ||
> -	    vrm->new_addr > TASK_SIZE - vrm->new_len)
> +	if (!vrm->new_len || vrm->new_len > TASK_SIZE)
>  		return -EINVAL;
>  
>  	/* Remainder of checks are for cases with specific new_addr. */
>  	if (!vrm_implies_new_addr(vrm))
>  		return 0;
>  
> +	/* Is the new address silly? */
> +	if (vrm->new_addr > TASK_SIZE - vrm->new_len)
> +		return -EINVAL;
> +
>  	/* The new address must be page-aligned. */
>  	if (offset_in_page(vrm->new_addr))
>  		return -EINVAL;
> -- 
> 2.51.0.268.g9569e192d0-goog
>
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
On Wed, Aug 27, 2025 at 11:43:39PM -0400, Liam R. Howlett wrote:
> * Carlos Llamas <cmllamas@google.com> [250827 23:27]:
> > Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> > checks") moved the sanity check for vrm->new_addr from mremap_to() to
> > check_mremap_params().
> > 
> > However, this caused a regression as vrm->new_addr is now checked even
> > when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> > case, vrm->new_addr can be garbage and create unexpected failures.
> > 
> > Fix this by moving the new_addr check after the vrm_implies_new_addr()
> > guard. This ensures that the new_addr is only checked when the user has
> > specified one explicitly.
> > 
> > Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> 
> I assume this showed up with clang?

Right.

The specific test that broke on our end was this:
https://android.googlesource.com/platform/bionic/+/HEAD/tests/__cxa_atexit_test.cpp
Although I'm not exactly sure how __cxa_atexit() implementation uses
mremap() underneath.
Re: [PATCH] mm/mremap: fix regression in vrm->new_addr check
Posted by Carlos Llamas 1 month ago 
On Thu, Aug 28, 2025 at 04:06:58AM +0000, Carlos Llamas wrote:
> On Wed, Aug 27, 2025 at 11:43:39PM -0400, Liam R. Howlett wrote:
> > * Carlos Llamas <cmllamas@google.com> [250827 23:27]:
> > > Commit 3215eaceca87 ("mm/mremap: refactor initial parameter sanity
> > > checks") moved the sanity check for vrm->new_addr from mremap_to() to
> > > check_mremap_params().
> > > 
> > > However, this caused a regression as vrm->new_addr is now checked even
> > > when MREMAP_FIXED and MREMAP_DONTUNMAP flags are not specified. In this
> > > case, vrm->new_addr can be garbage and create unexpected failures.
> > > 
> > > Fix this by moving the new_addr check after the vrm_implies_new_addr()
> > > guard. This ensures that the new_addr is only checked when the user has
> > > specified one explicitly.
> > > 
> > > Fixes: 3215eaceca87 ("mm/mremap: refactor initial parameter sanity checks")
> > > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > 
> > I assume this showed up with clang?
> 
> Right.
> 
> The specific test that broke on our end was this:
> https://android.googlesource.com/platform/bionic/+/HEAD/tests/__cxa_atexit_test.cpp
> Although I'm not exactly sure how __cxa_atexit() implementation uses
> mremap() underneath.

Ok, here is the specific call to mremap() in the test:
https://android.googlesource.com/platform/bionic/+/HEAD/libc/bionic/atexit.cpp#197

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.