[PATCH 0/2] mm/damon/core: validate damos_quota_goal->nid

[PATCH 0/2] mm/damon/core: validate damos_quota_goal->nid

[SeongJae Park]

node_mem[cg]_{used,free}_bp DAMOS quota goals receive the node id.  The
node id is used for si_meminfo_node() and NODE_DATA() without proper
validation.  As a result, privileged users can trigger an out of bounds
memory access using DAMON_SYSFS.  Fix the issues.

The issue was originally reported [1] with a fix by another author.  The
original author announced [2] that they will stop working including the
fix that was still in the review stage.  Hence I'm restarting this.

[1] https://lore.kernel.org/20260325073034.140353-1-objecting@objecting.org
[2] https://lore.kernel.org/20260327040924.68553-1-sj@kernel.org

Changes from RFC
(https://lore.kernel.org/20260328005412.7606-1-sj@kernel.org)
- fix typo in patch 2: s/MEM/MEMCG/.
- rebase to latest mm-new.

SeongJae Park (2):
  mm/damon/core: validate damos_quota_goal->nid for
    node_mem_{used,free}_bp
  mm/damon/core: validate damos_quota_goal->nid for
    node_memcg_{used,free}_bp

 mm/damon/core.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)


base-commit: 2f8cc7995d75c89079c55a85fc1d3092ffb7bd59
-- 
2.47.3

[PATCH 1/2] mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp

[SeongJae Park]

Users can set damos_quota_goal->nid with arbitrary value for
node_mem_{used,free}_bp.  But DAMON core is using those for
si_meminfo_node() without the validation of the value.  This can result
in out of bounds memory access.  The issue can actually triggered using
DAMON user-space tool (damo), like below.

    $ sudo ./damo start --damos_action stat \
    	--damos_quota_goal node_mem_used_bp 50% -1 \
    	--damos_quota_interval 1s
    $ sudo dmesg
    [...]
    [   65.565986] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000098

Fix this issue by adding the validation of the given node.  If an
invalid node id is given, it returns 0% for used memory ratio, and 100%
for free memory ratio.

Fixes: 0e1c773b501f ("mm/damon/core: introduce damos quota goal metrics for memory node utilization")
Cc: <stable@vger.kernel.org> # 6.16.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index ddabb93f2377..9a848d7647ef 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2217,12 +2217,24 @@ static inline u64 damos_get_some_mem_psi_total(void)
 #endif	/* CONFIG_PSI */
 
 #ifdef CONFIG_NUMA
+static bool invalid_mem_node(int nid)
+{
+	return nid < 0 || nid >= MAX_NUMNODES || !node_state(nid, N_MEMORY);
+}
+
 static __kernel_ulong_t damos_get_node_mem_bp(
 		struct damos_quota_goal *goal)
 {
 	struct sysinfo i;
 	__kernel_ulong_t numerator;
 
+	if (invalid_mem_node(goal->nid)) {
+		if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
+			return 0;
+		else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
+			return 10000;
+	}
+
 	si_meminfo_node(&i, goal->nid);
 	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
 		numerator = i.totalram - i.freeram;
-- 
2.47.3

[PATCH 2/2] mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp

[SeongJae Park]

Users can set damos_quota_goal->nid with arbitrary value for
node_memcg_{used,free}_bp.  But DAMON core is using those for
NODE-DATA() without a validation of the value.  This can result in out
of bounds memory access.  The issue can actually triggered using DAMON
user-space tool (damo), like below.

    $ sudo mkdir /sys/fs/cgroup/foo
    $ sudo ./damo start --damos_action stat --damos_quota_interval 1s \
            --damos_quota_goal node_memcg_used_bp 50% -1 /foo
    $ sudo dmseg
    [...]
    [  524.181426] Unable to handle kernel paging request at virtual address 0000000000002c00

Fix this issue by adding the validation of the given node id.  If an
invalid node id is given, it returns 0% for used memory ratio, and 100%
for free memory ratio.

Fixes: b74a120bcf50 ("mm/damon/core: implement DAMOS_QUOTA_NODE_MEMCG_USED_BP")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 9a848d7647ef..19642c175568 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2251,6 +2251,13 @@ static unsigned long damos_get_node_memcg_used_bp(
 	unsigned long used_pages, numerator;
 	struct sysinfo i;
 
+	if (invalid_mem_node(goal->nid)) {
+		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
+			return 0;
+		else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
+			return 10000;
+	}
+
 	memcg = mem_cgroup_get_from_id(goal->memcg_id);
 	if (!memcg) {
 		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
-- 
2.47.3

Re: (sashiko status) [PATCH 0/2] mm/damon/core: validate damos_quota_goal->nid

[SeongJae Park]

Forwarding sashiko.dev review status for this thread.

# review url: https://sashiko.dev/#/patchset/20260329043902.46163-1-sj@kernel.org

- [PATCH 1/2] mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp
  - status: Reviewed
  - review: ISSUES MAY FOUND
- [PATCH 2/2] mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp
  - status: Reviewed
  - review: ISSUES MAY FOUND

# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --thread_status --for_forwarding \
#             20260329043902.46163-1-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)

Re: (sashiko review) [PATCH 1/2] mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp

[SeongJae Park]

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260329043902.46163-2-sj@kernel.org
# start of sashiko.dev inline review
commit 8c40c45c57722526c25cf1114b386d19a693fc2d
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp

This commit adds validation for the node ID in damos_quota_goal to prevent
out of bounds memory access when calling si_meminfo_node(). If an invalid
node ID is given, it defaults to returning 0% for used memory and 100% for
free memory.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

>  static __kernel_ulong_t damos_get_node_mem_bp(
>  		struct damos_quota_goal *goal)
>  {
>  	struct sysinfo i;
>  	__kernel_ulong_t numerator;
>  
> +	if (invalid_mem_node(goal->nid)) {
> +		if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> +			return 0;
> +		else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> +			return 10000;
> +	}
> +
>  	si_meminfo_node(&i, goal->nid);
>  	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
>  		numerator = i.totalram - i.freeram;
>  	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
>  		numerator = i.freeram;
>  	return mult_frac(numerator, 10000, i.totalram);

Does this code still risk a divide-by-zero kernel panic if a memory node
has zero managed pages?

Even with the new invalid_mem_node() check ensuring node_state(nid, N_MEMORY)
is true, a memory node can have present physical pages but zero managed pages
if all its memory is reserved early in boot (for example, via memblock_reserve
for crash kernels or firmware).

In this scenario, si_meminfo_node() sets i.totalram to 0. Since the
mult_frac() macro directly evaluates division and modulo, passing
i.totalram as 0 will cause a hardware divide-by-zero exception.

Should there be an explicit check for i.totalram == 0 before calling
mult_frac() here and in other similar quota functions like
damos_get_node_memcg_used_bp()?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260329043902.46163-2-sj@kernel.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260329043902.46163-2-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)

Re: (sashiko review) [PATCH 2/2] mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp

[SeongJae Park]

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260329043902.46163-3-sj@kernel.org
# start of sashiko.dev inline review
commit 1ede2d837f5cbf8e8d96fe4c6eab34aeaa071d56
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp

This commit adds validation for the node id in damos_get_node_memcg_used_bp()
to prevent out-of-bounds memory access. If an invalid node id is provided, it
now safely returns 0% for used memory and 100% for free memory.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 26bdeb658799..c45c62f8449d 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2273,6 +2273,13 @@ static unsigned long damos_get_node_memcg_used_bp(
>  	unsigned long used_pages, numerator;
>  	struct sysinfo i;
>  
> +	if (invalid_mem_node(goal->nid)) {
> +		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> +			return 0;
> +		else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> +			return 10000;
> +	}
> +
>  	memcg = mem_cgroup_get_from_id(goal->memcg_id);
>  	if (!memcg) {
>  		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)

This isn't a bug introduced by this patch, but can the subtraction later in
this function underflow?

damos_get_node_memcg_used_bp() {
    ...
    else    /* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
        numerator = i.totalram - used_pages;
    ...
}

Since used_pages is fetched locklessly and i.totalram is the global managed
pages count, could asynchronous rstat accounting lag or concurrent memory
hot-remove operations cause used_pages to transiently exceed i.totalram?

Also regarding the end of this function, is it possible for i.totalram to
evaluate to 0 here?

damos_get_node_memcg_used_bp() {
    ...
    return mult_frac(numerator, 10000, i.totalram);
}

If a node's memory is entirely reserved or concurrently offlined,
si_meminfo_node() might set i.totalram to 0. If that happens, does
mult_frac() execute an unchecked division by zero internally?

Finally, does the mult_frac() macro overflow unsigned long on 32-bit
architectures?

The macro evaluates to q * n_ + r * n_ / d_, where r is
numerator % i.totalram. On 32-bit systems, r is a 32-bit unsigned long.

For nodes with more than ~1.7GB of RAM (where i.totalram exceeds ~429,496
pages), could r * 10000 silently overflow the 32-bit boundary and return a
severely truncated value?


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260329043902.46163-3-sj@kernel.org
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260329043902.46163-3-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail

Sent using hkml (https://github.com/sjp38/hackermail)

Re: (sashiko review) [PATCH 1/2] mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp

[SeongJae Park]

On Sun, 29 Mar 2026 08:34:06 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260329043902.46163-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 8c40c45c57722526c25cf1114b386d19a693fc2d
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp
> 
> This commit adds validation for the node ID in damos_quota_goal to prevent
> out of bounds memory access when calling si_meminfo_node(). If an invalid
> node ID is given, it defaults to returning 0% for used memory and 100% for
> free memory.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> >  static __kernel_ulong_t damos_get_node_mem_bp(
> >  		struct damos_quota_goal *goal)
> >  {
> >  	struct sysinfo i;
> >  	__kernel_ulong_t numerator;
> >  
> > +	if (invalid_mem_node(goal->nid)) {
> > +		if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> > +			return 0;
> > +		else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> > +			return 10000;
> > +	}
> > +
> >  	si_meminfo_node(&i, goal->nid);
> >  	if (goal->metric == DAMOS_QUOTA_NODE_MEM_USED_BP)
> >  		numerator = i.totalram - i.freeram;
> >  	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> >  		numerator = i.freeram;
> >  	return mult_frac(numerator, 10000, i.totalram);
> 
> Does this code still risk a divide-by-zero kernel panic if a memory node
> has zero managed pages?
> 
> Even with the new invalid_mem_node() check ensuring node_state(nid, N_MEMORY)
> is true, a memory node can have present physical pages but zero managed pages
> if all its memory is reserved early in boot (for example, via memblock_reserve
> for crash kernels or firmware).
> 
> In this scenario, si_meminfo_node() sets i.totalram to 0. Since the
> mult_frac() macro directly evaluates division and modulo, passing
> i.totalram as 0 will cause a hardware divide-by-zero exception.
> 
> Should there be an explicit check for i.totalram == 0 before calling
> mult_frac() here and in other similar quota functions like
> damos_get_node_memcg_used_bp()?

As I mentioned to same previous comment, that's another bug that require
another patch.  I will work on it with a separate patch.


Thanks,
SJ

[...]

Re: (sashiko review) [PATCH 2/2] mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp

[SeongJae Park]

TL; DR: sashiko findings are good points but not directly related to this
patch.  I will separately work on sashiko's findings.

On Sun, 29 Mar 2026 08:34:24 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260329043902.46163-3-sj@kernel.org
> # start of sashiko.dev inline review
> commit 1ede2d837f5cbf8e8d96fe4c6eab34aeaa071d56
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp
> 
> This commit adds validation for the node id in damos_get_node_memcg_used_bp()
> to prevent out-of-bounds memory access. If an invalid node id is provided, it
> now safely returns 0% for used memory and 100% for free memory.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 26bdeb658799..c45c62f8449d 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2273,6 +2273,13 @@ static unsigned long damos_get_node_memcg_used_bp(
> >  	unsigned long used_pages, numerator;
> >  	struct sysinfo i;
> >  
> > +	if (invalid_mem_node(goal->nid)) {
> > +		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> > +			return 0;
> > +		else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +			return 10000;
> > +	}
> > +
> >  	memcg = mem_cgroup_get_from_id(goal->memcg_id);
> >  	if (!memcg) {
> >  		if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> 
> This isn't a bug introduced by this patch, but can the subtraction later in
> this function underflow?
> 
> damos_get_node_memcg_used_bp() {
>     ...
>     else    /* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
>         numerator = i.totalram - used_pages;
>     ...
> }
> 
> Since used_pages is fetched locklessly and i.totalram is the global managed
> pages count, could asynchronous rstat accounting lag or concurrent memory
> hot-remove operations cause used_pages to transiently exceed i.totalram?

As sashiko also mentions, this is not related to this patch.  I will separately
work on this.

> 
> Also regarding the end of this function, is it possible for i.totalram to
> evaluate to 0 here?
> 
> damos_get_node_memcg_used_bp() {
>     ...
>     return mult_frac(numerator, 10000, i.totalram);
> }
> 
> If a node's memory is entirely reserved or concurrently offlined,
> si_meminfo_node() might set i.totalram to 0. If that happens, does
> mult_frac() execute an unchecked division by zero internally?

Again, not related to this patch.  I will separately work on this.

> 
> Finally, does the mult_frac() macro overflow unsigned long on 32-bit
> architectures?
> 
> The macro evaluates to q * n_ + r * n_ / d_, where r is
> numerator % i.totalram. On 32-bit systems, r is a 32-bit unsigned long.
> 
> For nodes with more than ~1.7GB of RAM (where i.totalram exceeds ~429,496
> pages), could r * 10000 silently overflow the 32-bit boundary and return a
> severely truncated value?

Good catch, but again, not related to this patch.  I will separtely work on
this.


Thanks,
SJ

[...]

Re: (sashiko status) [PATCH 0/2] mm/damon/core: validate damos_quota_goal->nid

[SeongJae Park]

On Sun, 29 Mar 2026 08:33:45 -0700 SeongJae Park <sj@kernel.org> wrote:

> Forwarding sashiko.dev review status for this thread.
> 
> # review url: https://sashiko.dev/#/patchset/20260329043902.46163-1-sj@kernel.org
> 
> - [PATCH 1/2] mm/damon/core: validate damos_quota_goal->nid for node_mem_{used,free}_bp
>   - status: Reviewed
>   - review: ISSUES MAY FOUND
> - [PATCH 2/2] mm/damon/core: validate damos_quota_goal->nid for node_memcg_{used,free}_bp
>   - status: Reviewed
>   - review: ISSUES MAY FOUND

TL; DR: sashiko shared good findings, but not directly related to this patch
series.  I will separately work on those.


Thanks,
SJ

[...]