On Fri, Dec 13, 2019 at 2:17 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Thu, Dec 12, 2019 at 9:50 PM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> I'll have my randconfig builder look for instances, so far I found one,
> see below. My feeling is that it would be better to enforce at least
> the size being a 1/2/4/8, to avoid cases where someone thinks
> the access is atomic, but it falls back on a memcpy.
>
> Arnd
>
> diff --git a/drivers/xen/time.c b/drivers/xen/time.c
> index 0968859c29d0..adb492c0aa34 100644
> --- a/drivers/xen/time.c
> +++ b/drivers/xen/time.c
> @@ -64,7 +64,7 @@ static void xen_get_runstate_snapshot_cpu_delta(
> do {
> state_time = get64(&state->state_entry_time);
> rmb(); /* Hypervisor might update data. */
> - *res = READ_ONCE(*state);
> + memcpy(res, state, sizeof(*res));
> rmb(); /* Hypervisor might update data. */
> } while (get64(&state->state_entry_time) != state_time ||
> (state_time & XEN_RUNSTATE_UPDATE));
A few hundred randconfig (x86, arm32 and arm64) builds later I
still only found one other instance:
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index eddae4688862..1c1f33447e96 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -304,7 +304,9 @@ static inline struct xdp_desc
*xskq_validate_desc(struct xsk_queue *q,
struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
unsigned int idx = q->cons_tail & q->ring_mask;
- *desc = READ_ONCE(ring->desc[idx]);
+ barrier();
+ memcpy(desc, &ring->desc[idx], sizeof(*desc));
+ barrier();
if (xskq_is_valid_desc(q, desc, umem))
return desc;
Arnd
