Re: [RFC PATCH] mm/damon/sysfs-schemes: hold damon_sysfs_lock in target_value_store()

[SJ Park]

On Tue, 20 Oct 2026 07:47:25 -0700 SJ Park <sj@xxxxxxxxxx> wrote:

> Remove a race condition under certain use cases that can trigger
> divide-by-zero.
> 
> damon_sysfs_add_quota_score() setup damos quota goal as user configured
> and pass to the core layer.  Because it could have a race with
> damos_sysfs_quota_goals_rm_dirs(), which destructs the files that carry
> user configuration, both are called under damon_sysfs_lock protection.
> 
> The damos quota goal setup disallows zero target_value parameter because
> it can be used as a denominator later in the core layer function,
> damos_quota_score().  Otherwise, divide-by-zero can happen.

This is no more true due to another patch [1].  The patch added the zero
target_value case in the core layer central validation pass.  But it forgot
removing the redundant check in sysfs.

> 
> For the zero value check and final assignment,
> damon_sysfs_add_quota_score() reads damos_sysfs_quota_goal->target_value
> twice.  And user can update damos_sysfs_quota_goal->target_value via
> target_value_store(), which doesn't hold damon_sysfs_lock.  As a result,
> a time-of-check-to time-of-use (toctou) race can happen.  If the user
> sets the value to zero between the zero value check and final
> target_value assignment, zero target_value is passed to the core layer
> and results in divide-by-zero.

So toctou race exists, but it cannot result in divide-by-zero.

> 
> Fix by holding damon_sysfs_lock in target_value_store() while saving the
> value.  The read side, damon_sysfs_add_quota_score(), is already called
> while holding the same mutex lock, so it avoids the race.
> 
> Users could reproduce the race by writing zero to the target_value sysfs
> file while DAMON is starting, or online parameters update is ongoing.
> The issue will only rarely happen in the real use cases.  It requires
> sysfs write permission.  The race window is short.  Sane users wouldn't
> write zero target_value.  Especially, they wouldn't do that while DAMON
> enablement and/or online parameters update is ongoing.  That said, the
> probability is not zero, and the consequence (divide-by-zero) is quite
> bad.
> 
> Closes: https://sashiko.dev/#/bug/linux-7f814c9a-986f-4103-a55a-ef9537dcf62b

Maybe Sashiko found the issue before the other patch [1] is applied.


> Fixes: 8b549a4fd3c5 ("mm/damon/sysfs-schemes: commit damos quota goals user input to DAMOS")
> Cc: <stable@xxxxxxxxxxxxxxx> # 6.8.x

So this patch is unnecessary for the mainline.  But still need to be added to
older kernels.  Backporting the mainline fix [1] would be one simple way.  The
backporting would be not that straightforward because the core layer parameters
validation was introduced on 7.3 via commit b90408ef1163 ("mm/damon/core:
safely validate src on damon_commit_ctx()").  Hopefully doable.  I will take
time to do backports for relavant LTS and stable kernels.

[1] https://lore.kernel.org/20260903010722.94244-2-sj@xxxxxxxxxx


Thanks,
SJ

[...]