Skip to contentSkip to conversation

FAILED: patch "[PATCH] ipc/sem.c: fix complex_count vs. simple op race" failed to apply to 4.7-stable tree

PATCH [email protected] 1 participant 1 message Oct 20, 2016 Stable branch

lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.7-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 5864a2fd3088db73d47942370d0f7210a807b9bc Mon Sep 17 00:00:00 2001

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

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.

Fixes 6d07b68ce16a ("ipc/sem.c: optimize sem_lock()")
Cc "H. Peter Anvin" <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Davidlohr Bueso <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc <[email protected]> [3.10+]
include/linux/sem.h +1 -0
index 976ce3a19f1b..d0efd6e6c20a 100644
@@ -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
ipc/sem.c +83 -55

220 lines not on this page · raw message