linux-kernel - Re: [PATCH] smp_wmb() in kcov_move

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]

Message-ID: <CAG_fn=VXnaMitRFpxP0Fjy=vWF+rjRfZ0TRsziwKzEVrArXt7g@mail.gmail.com>
Date: Thu, 17 Jul 2025 10:02:30 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Marco Elver <elver@...gle.com>
Cc: Soham Bagchi <sohambagchi@...look.com>, dvyukov@...gle.com, andreyknvl@...il.com, 
	akpm@...ux-foundation.org, tglx@...utronix.de, arnd@...db.de, 
	kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] smp_wmb() in kcov_move_area() after memcpy()

On Thu, Jul 17, 2025 at 9:16 AM Marco Elver <elver@...gle.com> wrote:
>
> [+Cc glider@...gle.com]
>
> On Thu, 17 Jul 2025 at 04:48, Soham Bagchi <sohambagchi@...look.com> wrote:
>
> Patch title should be something like "kcov: use write barrier after
> memcpy() in kcov_move_area()".
>
> > KCOV Remote uses two separate memory buffers, one private to the kernel
> > space (kcov_remote_areas) and the second one shared between user and
> > kernel space (kcov->area). After every pair of kcov_remote_start() and
> > kcov_remote_stop(), the coverage data collected in the
> > kcov_remote_areas is copied to kcov->area so the user can read the
> > collected coverage data. This memcpy() is located in kcov_move_area().
> >
> > The load/store pattern on the kernel-side [1] is:
> >
> > ```
> > /* dst_area === kcov->area, dst_area[0] is where the count is stored */
> > dst_len = READ_ONCE(*(unsigned long *)dst_area);
> > ...
> > memcpy(dst_entries, src_entries, ...);
> > ...
> > WRITE_ONCE(*(unsigned long *)dst_area, dst_len + entries_moved);
> > ```
> >
> > And for the user [2]:
> >
> > ```
> > /* cover is equivalent to kcov->area */
> > n = __atomic_load_n(&cover[0], __ATOMIC_RELAXED);

We shouldn't probably suggest the users to use relaxed loads either.

> > ```
> >
> > Without a write-memory barrier, the atomic load for the user can
> > potentially read fresh values of the count stored at cover[0],
> > but continue to read stale coverage data from the buffer itself.
> > Hence, we recommend adding a write-memory barrier between the
> > memcpy() and the WRITE_ONCE() in kcov_move_area().
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/kernel/kcov.c?h=master#n978
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/dev-tools/kcov.rst#n364
> >
> > Signed-off-by: Soham Bagchi <sohambagchi@...look.com>
Reviewed-by: Alexander Potapenko <glider@...gle.com>