Patch "ipc/sem.c: fix complex_count vs. simple op race" has been added to the 4.8-stable tree

This is a note to let you know that I've just added the patch titled

ipc/sem.c: fix complex_count vs. simple op race

to the 4.8-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
ipc-sem.c-fix-complex_count-vs.-simple-op-race.patch
and it can be found in the queue-4.8 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: Manfred Spraul <[email protected]>
Date: Tue, 11 Oct 2016 13:54:50 -0700
Subject: ipc/sem.c: fix complex_count vs. simple op race

From: Manfred Spraul <[email protected]>

commit 5864a2fd3088db73d47942370d0f7210a807b9bc upstream.

Commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") introduced a race:

sem_lock has a fast path that allows parallel simple operations. There are two reasons why a simple operation cannot run in parallel:
- a non-simple operations is ongoing (sma->sem_perm.lock held)
- a complex operation is sleeping (sma->complex_count != 0)

As both facts are stored independently, a thread can bypass the current checks by sleeping in the right positions. See below for more details (or kernel bugzilla 105651).

The patch fixes that by creating one variable (complex_mode) that tracks both reasons why parallel operations are not possible.

The patch also updates stale documentation regarding the locking.

With regards to stable kernels:
The patch is required for all kernels that include the commit 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()") (3.10?)

The alternative is to revert the patch that introduced the race.

The patch is safe for backporting, i.e. it makes no assumptions about memory barriers in spin_unlock_wait().

Background:
Here is the race of the current implementation:

Thread A: (simple op)
- does the first "sma->complex_count == 0" test

Thread B: (complex op)
- does sem_lock(): This includes an array scan. But the scan can't find Thread A, because Thread A does not own sem->lock yet.
- the thread does the operation, increases complex_count, drops sem_lock, sleeps

Thread A:
- spin_lock(&sem->lock), spin_is_locked(sma->sem_perm.lock)
- sleeps before the complex_count test

Thread C: (complex op)
- does sem_lock (no array scan, complex_count==1)
- wakes up Thread B.
- decrements complex_count

Thread A:
- does the complex_count test

Bug:
Now both thread A and thread C operate on the same array, without any synchronization.

---
 include/linux/sem.h |    1 
 ipc/sem.c           |  138 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 84 insertions(+), 55 deletions(-)

@@ -21,6 +21,7 @@ struct sem_array {
 	struct list_head	list_id;	/* undo requests on this array */
 	int			sem_nsems;	/* no. of semaphores in array */
 	int			complex_count;	/* pending complex operations */
+	bool			complex_mode;	/* no parallel simple ops */
 };
 
 #ifdef CONFIG_SYSVIPC

Patches currently in stable-queue which might be from [email protected] are

queue-4.8/ipc-sem.c-fix-complex_count-vs.-simple-op-race.patch