linux-kernel - Re: KCSAN: data-race in cgroup_rstat_flush_locked / cgroup_rstat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 20 Sep 2021 20:09:56 +0200
From:   Michal Koutný <mkoutny@...e.com>
To:     Hao Sun <sunhao.th@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        cgroups@...r.kernel.org, hannes@...xchg.org,
        lizefan.x@...edance.com, tj@...nel.org
Subject: Re: KCSAN: data-race in cgroup_rstat_flush_locked /
 cgroup_rstat_updated

On Sat, Sep 18, 2021 at 09:27:08AM +0800, Hao Sun <sunhao.th@...il.com> wrote:
> Would it be better to add a `data_race` macro to the corresponding
> location so that the false report can be disabled?

Something like this

--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -35,7 +35,7 @@ void cgroup_rstat_updated(struct cgroup *cgrp, int cpu)
         * instead of NULL, we can tell whether @cgrp is on the list by
         * testing the next pointer for NULL.
         */
-       if (cgroup_rstat_cpu(cgrp, cpu)->updated_next)
+       if (data_race(cgroup_rstat_cpu(cgrp, cpu)->updated_next))
                return;

        raw_spin_lock_irqsave(cpu_lock, flags);
?

Makes sense to me. Will you send a patch (if this resolves your KCSAN
noise)?

(IIUC, this becase more visible after commit aa48e47e3906 ("memcg:
infrastructure to flush memcg stats") v5.15-rc1 but it was present since
d8ef4b38cb69 ("Revert "cgroup: Add memory barriers to plug
cgroup_rstat_updated() race window"") v5.7.)

> See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/memory-model/Documentation/access-marking.txt#n58
> for more details.

(Interesting, learning...)

Thanks,
Michal