Re: [PATCH] mm: handle potential NULL return from anon_vma_name_reuse()

[Lorenzo Stoakes]

NAK, expected allocation failures (even if practically impossible) should not
cause arbitrary kernel warnings.

But also this is very silly, you would need INT_MAX references on the anon_name
to happen first...

On Tue, Apr 21, 2026 at 04:50:55PM +0800, Ye Liu wrote:
> From: Ye Liu <liuye@xxxxxxxxxx>
>
> The anon_vma_name_reuse() function may return NULL if memory allocation
> fails in anon_vma_name_alloc(). Currently, callers dup_anon_vma_name()
> and replace_anon_vma_name() do not check the return value, which could
> lead to NULL pointer dereferences.

We assign NULL if the allocation failed. And every code path understands
vma->anon_vma_name being NULL to be a valid situation of that VMA not having a
name.

In the case you're failing an allocation that small, the system is under extreme
memory pressure.

Not propagating an anon_vma_name, which is cosmetic, is totally fine under those
circumstances.

But also, you'd require the anon_vma_name to be saturated at REFCOUNT_MAX ==
INT_MAX.

So this is just silly.

>
> This patch adds proper error handling:
> - In dup_anon_vma_name(), if anon_vma_name_reuse() returns NULL, emit a
>   warning via WARN_ON_ONCE(1) since this is an unexpected condition.

It's not? Your whole thesis is that allocation failures can happen here and
cause a problem, so you can't simultaneously say it's 'unexpected'?

> - In replace_anon_vma_name(), return -ENOMEM to propagate the allocation
>   failure to the caller.
>
> These changes improve robustness against memory allocation failures.

No, they add a bunch of noise, then make an allocation failure into an
arbitrary, new kernel warning.

>
> Signed-off-by: Ye Liu <liuye@xxxxxxxxxx>
> ---
>  include/linux/mm_inline.h | 12 +++++++++---
>  mm/madvise.c              |  7 ++++++-
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index a171070e15f0..9bbaf8287806 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -421,9 +421,15 @@ static inline void dup_anon_vma_name(struct vm_area_struct *orig_vma,
>  				     struct vm_area_struct *new_vma)
>  {
>  	struct anon_vma_name *anon_name = anon_vma_name(orig_vma);
> -
> -	if (anon_name)
> -		new_vma->anon_name = anon_vma_name_reuse(anon_name);
> +	struct anon_vma_name *new_name;
> +
> +	if (anon_name) {
> +		new_name = anon_vma_name_reuse(anon_name);
> +		if (new_name)
> +			new_vma->anon_name = new_name;
> +		else
> +			WARN_ON_ONCE(1);
> +	}

This is horrible code, but as I said above, this is not only wrong it's actively
bad - you're causing an allocation failure to result in a kernel warning.

We don't mind anon_vma_name not being propagated after billions of references
and under maximally extreme memory pressure/fatal signal propagation.

>  }
>
>  static inline void free_anon_vma_name(struct vm_area_struct *vma)
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 69708e953cf5..ccb937a37e70 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -118,6 +118,7 @@ static int replace_anon_vma_name(struct vm_area_struct *vma,
>  				 struct anon_vma_name *anon_name)
>  {
>  	struct anon_vma_name *orig_name = anon_vma_name(vma);
> +	struct anon_vma_name *new_name;
>
>  	if (!anon_name) {
>  		vma->anon_name = NULL;
> @@ -128,7 +129,11 @@ static int replace_anon_vma_name(struct vm_area_struct *vma,
>  	if (anon_vma_name_eq(orig_name, anon_name))
>  		return 0;
>
> -	vma->anon_name = anon_vma_name_reuse(anon_name);
> +	new_name = anon_vma_name_reuse(anon_name);
> +	if (!new_name)
> +		return -ENOMEM;
> +
> +	vma->anon_name = new_name;

This is pointless noise. Billions of references, extreme memory pressure
(practically impossible anyway).

>  	anon_vma_name_put(orig_name);
>
>  	return 0;
> --
> 2.43.0
>

Thanks, Lorenzo