| Message ID | 1582302724-2804-1-git-send-email-cai@lca.pw (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | percpu_counter: fix a data race at vm_committed_as | expand |
On Fri, 21 Feb 2020, Qian Cai wrote: > "vm_committed_as.count" could be accessed concurrently as reported by > KCSAN, > > BUG: KCSAN: data-race in __vm_enough_memory / percpu_counter_add_batch > > write to 0xffffffff9451c538 of 8 bytes by task 65879 on cpu 35: > percpu_counter_add_batch+0x83/0xd0 > percpu_counter_add_batch at lib/percpu_counter.c:91 > __vm_enough_memory+0xb9/0x260 > dup_mm+0x3a4/0x8f0 > copy_process+0x2458/0x3240 > _do_fork+0xaa/0x9f0 > __do_sys_clone+0x125/0x160 > __x64_sys_clone+0x70/0x90 > do_syscall_64+0x91/0xb05 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > read to 0xffffffff9451c538 of 8 bytes by task 66773 on cpu 19: > __vm_enough_memory+0x199/0x260 > percpu_counter_read_positive at include/linux/percpu_counter.h:81 > (inlined by) __vm_enough_memory at mm/util.c:839 > mmap_region+0x1b2/0xa10 > do_mmap+0x45c/0x700 > vm_mmap_pgoff+0xc0/0x130 > ksys_mmap_pgoff+0x6e/0x300 > __x64_sys_mmap+0x33/0x40 > do_syscall_64+0x91/0xb05 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > The read is outside percpu_counter::lock critical section which results > in a data race. Fix it by adding a READ_ONCE() in > percpu_counter_read_positive() which could also service as the existing > compiler memory barrier. > > Signed-off-by: Qian Cai <cai@lca.pw> Acked-by: Marco Elver <elver@google.com> FWIW in the function where this was inlined here, the generated code (on x86 at least) is identical. Thanks, -- Marco > --- > include/linux/percpu_counter.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/percpu_counter.h b/include/linux/percpu_counter.h > index 4f052496cdfd..0a4f54dd4737 100644 > --- a/include/linux/percpu_counter.h > +++ b/include/linux/percpu_counter.h > @@ -78,9 +78,9 @@ static inline s64 percpu_counter_read(struct percpu_counter *fbc) > */ > static inline s64 percpu_counter_read_positive(struct percpu_counter *fbc) > { > - s64 ret = fbc->count; > + /* Prevent reloads of fbc->count */ > + s64 ret = READ_ONCE(fbc->count); > > - barrier(); /* Prevent reloads of fbc->count */ > if (ret >= 0) > return ret; > return 0; > -- > 1.8.3.1 >
diff --git a/include/linux/percpu_counter.h b/include/linux/percpu_counter.h index 4f052496cdfd..0a4f54dd4737 100644 --- a/include/linux/percpu_counter.h +++ b/include/linux/percpu_counter.h @@ -78,9 +78,9 @@ static inline s64 percpu_counter_read(struct percpu_counter *fbc) */ static inline s64 percpu_counter_read_positive(struct percpu_counter *fbc) { - s64 ret = fbc->count; + /* Prevent reloads of fbc->count */ + s64 ret = READ_ONCE(fbc->count); - barrier(); /* Prevent reloads of fbc->count */ if (ret >= 0) return ret; return 0;
"vm_committed_as.count" could be accessed concurrently as reported by KCSAN, BUG: KCSAN: data-race in __vm_enough_memory / percpu_counter_add_batch write to 0xffffffff9451c538 of 8 bytes by task 65879 on cpu 35: percpu_counter_add_batch+0x83/0xd0 percpu_counter_add_batch at lib/percpu_counter.c:91 __vm_enough_memory+0xb9/0x260 dup_mm+0x3a4/0x8f0 copy_process+0x2458/0x3240 _do_fork+0xaa/0x9f0 __do_sys_clone+0x125/0x160 __x64_sys_clone+0x70/0x90 do_syscall_64+0x91/0xb05 entry_SYSCALL_64_after_hwframe+0x49/0xbe read to 0xffffffff9451c538 of 8 bytes by task 66773 on cpu 19: __vm_enough_memory+0x199/0x260 percpu_counter_read_positive at include/linux/percpu_counter.h:81 (inlined by) __vm_enough_memory at mm/util.c:839 mmap_region+0x1b2/0xa10 do_mmap+0x45c/0x700 vm_mmap_pgoff+0xc0/0x130 ksys_mmap_pgoff+0x6e/0x300 __x64_sys_mmap+0x33/0x40 do_syscall_64+0x91/0xb05 entry_SYSCALL_64_after_hwframe+0x49/0xbe The read is outside percpu_counter::lock critical section which results in a data race. Fix it by adding a READ_ONCE() in percpu_counter_read_positive() which could also service as the existing compiler memory barrier. Signed-off-by: Qian Cai <cai@lca.pw> --- include/linux/percpu_counter.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)