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