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

SJ Park <[email protected]>
Newsgroups dev.linux.lists.damon
Message-ID <[email protected]>
On Tue, 14 Jul 2026 14:25:08 +0000 [email protected] wrote:

> 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?

Good catch.  I also found I forgot adding Fixes: tag.  I'll fix this in the
next revision.

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


Thanks,
SJ
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.