Re: [PATCH 5/5] mm/damon/sysfs: read ops_id only once

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch implements an incomplete fix for a TOCTOU data race. It correctly uses READ_ONCE() for `ops_id` to prevent `min_region_sz` corruption, but leaves an identical unresolved race condition for `addr_unit` immediately below it.
--

--- Patch [5]: [PATCH 5/5] mm/damon/sysfs: read ops_id only once ---
commit f0f9d921acb64429b2f4e2895a8088cc3b5477d5
Author: SJ Park <[email protected]>

mm/damon/sysfs: read ops_id only once

This commit addresses a time-of-check to time-of-use race condition in
damon_sysfs_apply_inputs() by reading ops_id only once. This prevents a
concurrent sysfs write from causing min_region_sz to be incorrectly
configured.

Link: https://lore.kernel.org/[email protected]

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index b5fe036f78015..60a1a9e4ada34 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -2094,14 +2094,16 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,

[ ... ]

>  	ctx->addr_unit = sys_ctx->addr_unit;
>  	/* addr_unit is respected by only DAMON_OPS_PADDR */
> -	if (sys_ctx->ops_id == DAMON_OPS_PADDR)
> +	if (ops_id == DAMON_OPS_PADDR)
>  		ctx->min_region_sz = max(
>  				DAMON_MIN_REGION_SZ / sys_ctx->addr_unit, 1);

[Severity: Medium]
Is there a similar race condition remaining for sys_ctx->addr_unit?

While this patch successfully fixes the race for ops_id, sys_ctx->addr_unit
is read twice sequentially in this same block of code without protection.

A concurrent sysfs write to addr_unit between the assignment to
ctx->addr_unit and the division in the max() calculation could cause DAMON
to inconsistently calculate min_region_sz, degrading monitoring precision.

Should addr_unit also be read only once using READ_ONCE() here to fully
prevent the race?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5