[PATCH] percpu_counter: annotate lockless read in _limited_add()

[Jose A. Perez de Azpillaga]

syzbot reports a data race on fbc->count between the lockless read in
the fast path of __percpu_counter_limited_add() and the locked update
in percpu_counter_add_batch(), reached via shmem's used_blocks counter:
the alloc path reads it through percpu_counter_limited_add() while the
free path updates it through percpu_counter_sub().

Annotate the read rather than change the logic: the lockless read is
deliberate, only the annotation is missing. It is an approximation,
not a conservative bound. A concurrent flush moves value between
fbc->count and a per-cpu counter, and other CPUs' locked slow paths add
to fbc->count too, so a stale low value can let the fast path proceed
where the slow path would refuse. The error is bounded by the per-cpu
slack (unknown = batch * num_online_cpus()). This is existing behavior,
no functional change intended.

Read it with data_race(READ_ONCE(fbc->count)): data_race() tells KCSAN
the race is intended (READ_ONCE() alone still triggers the report,
because KCSAN reports against watchpoints set up by plain accesses),
while READ_ONCE() stops the compiler from refetching the value, as in
percpu_counter_read_positive(). The lock-protected accesses stay plain,
so future buggy lockless writes are still caught.

Reported-by: syzbot+a3c71b9db9c11c270f59@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=a3c71b9db9c11c270f59
Signed-off-by: Jose A. Perez de Azpillaga <azpijr@xxxxxxxxx>
---
Tested with a KCSAN kernel: defconfig + CONFIG_KCSAN=y, CONFIG_KASAN=n,
CONFIG_KCSAN_SKIP_WATCH=200, CONFIG_KCSAN_UDELAY_TASK=200, KCSAN left off
at boot and enabled from init via debugfs; 8 vCPU / 4G QEMU/KVM guest,
gcc 16.2.1.  Stress: 120 s of concurrent write() and unlink()/ftruncate()
on one size-limited tmpfs (shmem_inode_acct_blocks vs
shmem_inode_unacct_blocks), which is the pair from the report.

	base/mm-new:  38x "BUG: KCSAN: data-race in __percpu_counter_limited_add
		/ percpu_counter_add_batch", first hit ~0.3 s after the
		workers started
	this commit:  0 percpu_counter reports in the same 120 s
		(other, unrelated KCSAN reports remain: d_make_discardable,
		osq_lock)

Object code (defconfig, CONFIG_KCSAN=n): __percpu_counter_limited_add() is
the same size (0x23b) with the same four fbc->count loads and identical
branch targets; .text differs only in register allocation and compare
operand order (45 of 2171 bytes).  No functional change intended.

 lib/percpu_counter.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c
index 2891f94a11c6..87b261b87b0b 100644
--- a/lib/percpu_counter.c
+++ b/lib/percpu_counter.c
@@ -328,6 +328,7 @@ bool __percpu_counter_limited_add(struct percpu_counter *fbc,
 				  s64 limit, s64 amount, s32 batch)
 {
 	s64 count;
+	s64 gcount;
 	s64 unknown;
 	unsigned long flags;
 	bool good = false;
@@ -338,11 +339,16 @@ bool __percpu_counter_limited_add(struct percpu_counter *fbc,
 	local_irq_save(flags);
 	unknown = batch * num_online_cpus();
 	count = __this_cpu_read(*fbc->counters);
+	/*
+	 * Lockless on purpose: gcount may be stale, so this is only an
+	 * approximation, bounded by the per-cpu slack ("unknown").
+	 */
+	gcount = data_race(READ_ONCE(fbc->count));

 	/* Skip taking the lock when safe */
 	if (abs(count + amount) <= batch &&
-	    ((amount > 0 && fbc->count + unknown <= limit) ||
-	     (amount < 0 && fbc->count - unknown >= limit))) {
+	    ((amount > 0 && gcount + unknown <= limit) ||
+	     (amount < 0 && gcount - unknown >= limit))) {
 		this_cpu_add(*fbc->counters, amount);
 		local_irq_restore(flags);
 		return true;
--
2.55.0