include/linux/mm_inline.h | 12 +++++++++--- mm/madvise.c | 7 ++++++- 2 files changed, 15 insertions(+), 4 deletions(-)
From: Ye Liu <liuye@kylinos.cn>
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.
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.
- In replace_anon_vma_name(), return -ENOMEM to propagate the allocation
failure to the caller.
These changes improve robustness against memory allocation failures.
Signed-off-by: Ye Liu <liuye@kylinos.cn>
---
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);
+ }
}
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;
anon_vma_name_put(orig_name);
return 0;
--
2.43.0NAK, 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@kylinos.cn>
>
> 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@kylinos.cn>
> ---
> 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
在 2026/4/21 17:08, Lorenzo Stoakes 写道:
> NAK, expected allocation failures (even if practically impossible) should not
> cause arbitrary kernel warnings.
Thank you both for the thorough review and explanation.
You are absolutely right. I mistakenly assumed that leaving vma->anon_name
as NULL could lead to a NULL pointer dereference in some path, but as you
pointed out, the code is designed to treat NULL as "no name", which is
perfectly valid.
I also acknowledge that adding a WARN_ON_ONCE in an allocation failure path
is incorrect and harmful under memory pressure.
Please disregard this patch. I've learned a valuable lesson about error
handling in cosmetic features.
Thank you again for the guidance.
--
Thanks,
Ye Liu
On 4/21/26 10:50, Ye Liu wrote:
> From: Ye Liu <liuye@kylinos.cn>
>
> 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.
When, where?
If we set vma->anon_name = NULL, it just looks like there is no anon
VMA, or what am I missing?
>
> 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.
> - In replace_anon_vma_name(), return -ENOMEM to propagate the allocation
> failure to the caller.
>
> These changes improve robustness against memory allocation failures.
>
> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> ---
> 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);
Triggering a warning when an allocation fails? Certainly not.
--
Cheers,
David
On Tue, Apr 21, 2026 at 10:55:16AM +0200, David Hildenbrand (Arm) wrote:
> On 4/21/26 10:50, Ye Liu wrote:
> > From: Ye Liu <liuye@kylinos.cn>
> >
> > 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.
>
> When, where?
>
> If we set vma->anon_name = NULL, it just looks like there is no anon
> VMA, or what am I missing?
>
> >
> > 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.
> > - In replace_anon_vma_name(), return -ENOMEM to propagate the allocation
> > failure to the caller.
> >
> > These changes improve robustness against memory allocation failures.
> >
> > Signed-off-by: Ye Liu <liuye@kylinos.cn>
> > ---
> > 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);
>
> Triggering a warning when an allocation fails? Certainly not.
It's pleasing to reply independently and see that we've reached the same
conclusions here :P
I mean, yeah agreed obviously!
>
> --
> Cheers,
>
> David
Cheers, Lorenzo
© 2016 - 2026 Red Hat, Inc.