ipc/sem.c: use READ_ONCE()/WRITE_ONCE() for use_global_lock

The patch solves two weaknesses in ipc/sem.c:

1) The initial read of use_global_lock in sem_lock() is an intentional
race.  KCSAN detects these accesses and prints a warning.

2) The code assumes that plain C read/writes are not mangled by the CPU or
the compiler.

To solve both issues, use READ_ONCE()/WRITE_ONCE().  Plain C reads are
used in code that owns sma->sem_perm.lock.

Link: https://lkml.kernel.org/r/20210514175319.12195-1-manfred@colorfullife.com
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Davidlohr Bueso <dbueso@suse.de>
Cc: <1vier1@web.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

diff --git a/ipc/sem.c b/ipc/sem.c
index 3a58188733d80e7dbccc4a6e3e44d6708e1f9dcd..e204bfc129f94c5c58502056071086d4b166aade 100644 (file)
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -217,6 +217,8 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
  * this smp_load_acquire(), this is guaranteed because the smp_load_acquire()
  * is inside a spin_lock() and after a write from 0 to non-zero a
  * spin_lock()+spin_unlock() is done.
+ * To prevent the compiler/cpu temporarily writing 0 to use_global_lock,
+ * READ_ONCE()/WRITE_ONCE() is used.
  *
  * 2) queue.status: (SEM_BARRIER_2)
  * Initialization is done while holding sem_lock(), so no further barrier is
@@ -342,10 +344,10 @@ static void complexmode_enter(struct sem_array *sma)
                 * Nothing to do, just reset the
                 * counter until we return to simple mode.
                 */
-               sma->use_global_lock = USE_GLOBAL_LOCK_HYSTERESIS;
+               WRITE_ONCE(sma->use_global_lock, USE_GLOBAL_LOCK_HYSTERESIS);
                return;
        }
-       sma->use_global_lock = USE_GLOBAL_LOCK_HYSTERESIS;
+       WRITE_ONCE(sma->use_global_lock, USE_GLOBAL_LOCK_HYSTERESIS);
 
        for (i = 0; i < sma->sem_nsems; i++) {
                sem = &sma->sems[i];
@@ -371,7 +373,8 @@ static void complexmode_tryleave(struct sem_array *sma)
                /* See SEM_BARRIER_1 for purpose/pairing */
                smp_store_release(&sma->use_global_lock, 0);
        } else {
-               sma->use_global_lock--;
+               WRITE_ONCE(sma->use_global_lock,
+                               sma->use_global_lock-1);
        }
 }
 
@@ -412,7 +415,7 @@ static inline int sem_lock(struct sem_array *sma, struct sembuf *sops,
         * Initial check for use_global_lock. Just an optimization,
         * no locking, no memory barrier.
         */
-       if (!sma->use_global_lock) {
+       if (!READ_ONCE(sma->use_global_lock)) {
                /*
                 * It appears that no complex operation is around.
                 * Acquire the per-semaphore lock.