linux-kernel - [PATCH 0/5] V3: Clarify/standardize memory barriers for ipc

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-next>] [day] [month] [year] [list]

Date:   Sun, 20 Oct 2019 14:33:00 +0200
From:   Manfred Spraul <manfred@...orfullife.com>
To:     LKML <linux-kernel@...r.kernel.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Waiman Long <longman@...hat.com>
Cc:     1vier1@....de, Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Manfred Spraul <manfred@...orfullife.com>
Subject: [PATCH 0/5] V3: Clarify/standardize memory barriers for ipc

Hi,

Updated series, based on input from Davidlohr and Peter Zijlstra:

- I've dropped the documentation update for wake_q_add, as what it
  states is normal: When you call a function and pass a parameter
  to a structure, you as caller are responsible to ensure that the 
  parameter is valid, and remains valid for the duration of the
  function call, including any tearing due to memory reordering.
  In addition, I've switched ipc to wake_q_add_safe().

- The patch to Documentation/memory_barriers.txt now as first change.
  @Davidlohr: You proposed to have 2 paragraphs: First, one for
  add/subtract, then one for failed cmpxchg. I didn't like that:
  We have one rule (can be combined with non-mb RMW ops), and then
  examples what are non-mb RMW ops. Listing special cases just ask
  for issues later.
  What I don't know is if there should be examples at all in
  Documentation/memory_barriers, or just
  "See Documentation/atomic_t.txt for examples of RMW ops that
  do not contain a memory barrier"

- For the memory barrier pairs in ipc/<whatever>, I have now added
  /* See ABC_BARRIER for purpose/pairing */ as standard comment,
  and then a block near the relevant structure where purpose, pairing
  races, ... are explained. I think this makes it easier to read,
  compared to adding it to both the _release and _acquire branches.

Description/purpose:

The memory barriers in ipc are not properly documented, and at least
for some architectures insufficient:
Reading the xyz->status is only a control barrier, thus
smp_acquire__after_ctrl_dep() was missing in mqueue.c and msg.c
sem.c contained a full smp_mb(), which is not required.

Patches:
Patch 1: Documentation for smp_mb__{before,after}_atomic().

Patch 2: Remove code duplication inside ipc/mqueue.c

Patch 3-5: Update the ipc code, especially add missing
           smp_mb__after_ctrl_dep() and switch to wake_q_add_safe().

Clarify that smp_mb__{before,after}_atomic() are compatible with all
RMW atomic operations, not just the operations that do not return a value.

Open issues:
- More testing. I did some tests, but doubt that the tests would be
  sufficient to show issues with regards to incorrect memory barriers.

What do you think?

--
	Manfred