Re: (sashiko review) [RFC 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/20260328005412.7606-2-sj@xxxxxxxxxx
# start of sashiko.dev inline review
commit add63112781ae04626ae6cb9bce55a85234f6e49
Author: SeongJae Park <sj@xxxxxxxxxx>

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

This commit adds validation for the given memory node id to prevent out of
bounds access when calculating node memory metrics. If an invalid node id
is provided, it defaults to returning 0% used or 100% free memory.

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

[ ... ]

> @@ -2239,12 +2239,24 @@ static inline u64 damos_get_some_mem_psi_total(void)
>  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);

Could this result in a division by zero panic?

While invalid_mem_node() checks node_state(nid, N_MEMORY) to ensure the
node has present memory pages, si_meminfo_node() sets i.totalram based
solely on managed pages. If all memory on the node is reserved, i.totalram
could be 0.

Additionally, since the node_state() check is lockless, memory could be
concurrently offlined after invalid_mem_node() but before si_meminfo_node(),
also resulting in i.totalram being 0.

Would it be safer to explicitly verify i.totalram is greater than 0 before
calling mult_frac()?


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

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