![Manfred Spraul](/assets/img/avatar_default.png)
Commit6d07b68ce1
("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 commit6d07b68ce1
("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:6d07b68ce1
("ipc/sem.c: optimize sem_lock()") Link: http://lkml.kernel.org/r/1469123695-5661-1-git-send-email-manfred@colorfullife.com Reported-by: <felixh@informatik.uni-bremen.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@elte.hu> Cc: <1vier1@web.de> Cc: <stable@vger.kernel.org> [3.10+] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
54 lines
1.3 KiB
C
54 lines
1.3 KiB
C
#ifndef _LINUX_SEM_H
|
|
#define _LINUX_SEM_H
|
|
|
|
#include <linux/atomic.h>
|
|
#include <linux/rcupdate.h>
|
|
#include <linux/cache.h>
|
|
#include <uapi/linux/sem.h>
|
|
|
|
struct task_struct;
|
|
|
|
/* One sem_array data structure for each set of semaphores in the system. */
|
|
struct sem_array {
|
|
struct kern_ipc_perm ____cacheline_aligned_in_smp
|
|
sem_perm; /* permissions .. see ipc.h */
|
|
time_t sem_ctime; /* last change time */
|
|
struct sem *sem_base; /* ptr to first semaphore in array */
|
|
struct list_head pending_alter; /* pending operations */
|
|
/* that alter the array */
|
|
struct list_head pending_const; /* pending complex operations */
|
|
/* that do not alter semvals */
|
|
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
|
|
|
|
struct sysv_sem {
|
|
struct sem_undo_list *undo_list;
|
|
};
|
|
|
|
extern int copy_semundo(unsigned long clone_flags, struct task_struct *tsk);
|
|
extern void exit_sem(struct task_struct *tsk);
|
|
|
|
#else
|
|
|
|
struct sysv_sem {
|
|
/* empty */
|
|
};
|
|
|
|
static inline int copy_semundo(unsigned long clone_flags, struct task_struct *tsk)
|
|
{
|
|
return 0;
|
|
}
|
|
|
|
static inline void exit_sem(struct task_struct *tsk)
|
|
{
|
|
return;
|
|
}
|
|
#endif
|
|
|
|
#endif /* _LINUX_SEM_H */
|