mm/damon/core: validate damos_quota_goal->nid

PATCH Applied SeongJae Park 1 participant 9 messages Mar 29

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

SeongJae Park

author maintainer

2026-03-29 04:39 UTC #0 lore raw

cover mm/damon/core: validate damos_quota_goal->nid

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/[email protected]
[2] https://lore.kernel.org/[email protected]

Changes from RFC
(https://lore.kernel.org/[email protected])
- 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

Signature · 1 line

2.47.3

SeongJae Park

author maintainer

2026-03-29 04:39 UTC #1 lore raw

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

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 <[email protected]> # 6.16.x

Signed-off-by SeongJae Park <[email protected]>

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

mm/damon/core.c +12 -0

index ddabb93f2377..9a848d7647ef 100644
@@ -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;

Signature · 1 line

2.47.3

SeongJae Park

author maintainer

2026-03-29 15:34 UTC #2 lore raw

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

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 8c40c45c57722526c25cf1114b386d19a693fc2d
Author: SeongJae Park <[email protected]>

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.

Quoted 3 lines

mm/damon/core.c +0 -0

[ ... ]

Quoted 9 lines

static __kernel_ulong_t damos_get_node_mem_bp(

mm/damon/core.c +3 -0

 		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;

mm/damon/core.c static inline u64 damos_get_some_mem_psi_total(void)

Quoted 10 lines

mm/damon/core.c +4 -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/[email protected]
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             [email protected]
#
# [1] https://github.com/sjp38/hackermail

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

SeongJae Park

author maintainer

2026-03-29 15:36 UTC #3 lore raw

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

Quoted 48 lines · On Sun, 29 Mar 2026 08:34:06 -0700 SeongJae Park <[email protected]> wrote:

Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 8c40c45c57722526c25cf1114b386d19a693fc2d
Author: SeongJae Park <[email protected]>
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.
Quoted 3 lines
mm/damon/core.c +0 -0
[ ... ]
Quoted 9 lines
static __kernel_ulong_t damos_get_node_mem_bp(
mm/damon/core.c +3 -0
 		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;
mm/damon/core.c static inline u64 damos_get_some_mem_psi_total(void)
Quoted 10 lines
mm/damon/core.c +4 -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).

Quoted 7 lines

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

[...]

SeongJae Park

author maintainer

2026-03-29 04:39 UTC #4 lore raw

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

In reply to 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 <[email protected]> # 6.19.x

Signed-off-by SeongJae Park <[email protected]>

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

mm/damon/core.c +7 -0

index 9a848d7647ef..19642c175568 100644
@@ -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)

Signature · 1 line

2.47.3

SeongJae Park

author maintainer

2026-03-29 15:34 UTC #5 lore raw

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

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 1ede2d837f5cbf8e8d96fe4c6eab34aeaa071d56
Author: SeongJae Park <[email protected]>

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.

Quoted 8 lines

mm/damon/core.c +0 -0

index 26bdeb658799..c45c62f8449d 100644
@@ -2273,6 +2273,13 @@ static unsigned long damos_get_node_memcg_used_bp(
 	unsigned long used_pages, numerator;
 	struct sysinfo i;

mm/damon/core.c static unsigned long damos_get_node_memcg_used_bp( · line 2285

Quoted 10 lines

mm/damon/core.c +7 -0

+	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/[email protected]
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             [email protected]
#
# [1] https://github.com/sjp38/hackermail

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

SeongJae Park

author maintainer

2026-03-29 15:48 UTC #6 lore raw

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

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

Quoted 42 lines · On Sun, 29 Mar 2026 08:34:24 -0700 SeongJae Park <[email protected]> wrote:

Forwarding Sashiko review for doing discussions via mails.
# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 1ede2d837f5cbf8e8d96fe4c6eab34aeaa071d56
Author: SeongJae Park <[email protected]>
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.
Quoted 8 lines
mm/damon/core.c +0 -0
index 26bdeb658799..c45c62f8449d 100644
@@ -2273,6 +2273,13 @@ static unsigned long damos_get_node_memcg_used_bp(
 	unsigned long used_pages, numerator;
 	struct sysinfo i;
 
mm/damon/core.c static unsigned long damos_get_node_memcg_used_bp( · line 2285
Quoted 10 lines
mm/damon/core.c +7 -0
+	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;
    ...
}

Quoted 3 lines

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.

Quoted 4 lines

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

Quoted 8 lines

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.

Quoted 10 lines

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

[...]

SeongJae Park

author maintainer

2026-03-29 15:33 UTC #7 lore raw

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

In reply to SeongJae Park

Forwarding sashiko.dev review status for this thread.

# review url: https://sashiko.dev/#/patchset/[email protected]

- [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 \
#             [email protected]
#
# [1] https://github.com/sjp38/hackermail

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

SeongJae Park

author maintainer

2026-03-29 16:30 UTC #8 lore raw

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

Quoted 10 lines · On Sun, 29 Mar 2026 08:33:45 -0700 SeongJae Park <[email protected]> wrote:

Forwarding sashiko.dev review status for this thread.
# review url: https://sashiko.dev/#/patchset/[email protected]
- [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

[...]