Revision edf28f4061afe4c2d9eb1c3323d90e882c1d6800 authored by Ioanna Alifieraki on 21 February 2020, 04:04:00 UTC, committed by Linus Torvalds on 21 February 2020, 19:22:15 UTC
This reverts commit a97955844807e327df11aa33869009d14d6b7de0.

Commit a97955844807 ("ipc,sem: remove uneeded sem_undo_list lock usage
in exit_sem()") removes a lock that is needed.  This leads to a process
looping infinitely in exit_sem() and can also lead to a crash.  There is
a reproducer available in [1] and with the commit reverted the issue
does not reproduce anymore.

Using the reproducer found in [1] is fairly easy to reach a point where
one of the child processes is looping infinitely in exit_sem between
for(;;) and if (semid == -1) block, while it's trying to free its last
sem_undo structure which has already been freed by freeary().

Each sem_undo struct is on two lists: one per semaphore set (list_id)
and one per process (list_proc).  The list_id list tracks undos by
semaphore set, and the list_proc by process.

Undo structures are removed either by freeary() or by exit_sem().  The
freeary function is invoked when the user invokes a syscall to remove a
semaphore set.  During this operation freeary() traverses the list_id
associated with the semaphore set and removes the undo structures from
both the list_id and list_proc lists.

For this case, exit_sem() is called at process exit.  Each process
contains a struct sem_undo_list (referred to as "ulp") which contains
the head for the list_proc list.  When the process exits, exit_sem()
traverses this list to remove each sem_undo struct.  As in freeary(),
whenever a sem_undo struct is removed from list_proc, it is also removed
from the list_id list.

Removing elements from list_id is safe for both exit_sem() and freeary()
due to sem_lock().  Removing elements from list_proc is not safe;
freeary() locks &un->ulp->lock when it performs
list_del_rcu(&un->list_proc) but exit_sem() does not (locking was
removed by commit a97955844807 ("ipc,sem: remove uneeded sem_undo_list
lock usage in exit_sem()").

This can result in the following situation while executing the
reproducer [1] : Consider a child process in exit_sem() and the parent
in freeary() (because of semctl(sid[i], NSEM, IPC_RMID)).

 - The list_proc for the child contains the last two undo structs A and
   B (the rest have been removed either by exit_sem() or freeary()).

 - The semid for A is 1 and semid for B is 2.

 - exit_sem() removes A and at the same time freeary() removes B.

 - Since A and B have different semid sem_lock() will acquire different
   locks for each process and both can proceed.

The bug is that they remove A and B from the same list_proc at the same
time because only freeary() acquires the ulp lock. When exit_sem()
removes A it makes ulp->list_proc.next to point at B and at the same
time freeary() removes B setting B->semid=-1.

At the next iteration of for(;;) loop exit_sem() will try to remove B.

The only way to break from for(;;) is for (&un->list_proc ==
&ulp->list_proc) to be true which is not. Then exit_sem() will check if
B->semid=-1 which is and will continue looping in for(;;) until the
memory for B is reallocated and the value at B->semid is changed.

At that point, exit_sem() will crash attempting to unlink B from the
lists (this can be easily triggered by running the reproducer [1] a
second time).

To prove this scenario instrumentation was added to keep information
about each sem_undo (un) struct that is removed per process and per
semaphore set (sma).

          CPU0                                CPU1
  [caller holds sem_lock(sma for A)]      ...
  freeary()                               exit_sem()
  ...                                     ...
  ...                                     sem_lock(sma for B)
  spin_lock(A->ulp->lock)                 ...
  list_del_rcu(un_A->list_proc)           list_del_rcu(un_B->list_proc)

Undo structures A and B have different semid and sem_lock() operations
proceed.  However they belong to the same list_proc list and they are
removed at the same time.  This results into ulp->list_proc.next
pointing to the address of B which is already removed.

After reverting commit a97955844807 ("ipc,sem: remove uneeded
sem_undo_list lock usage in exit_sem()") the issue was no longer
reproducible.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1694779

Link: http://lkml.kernel.org/r/20191211191318.11860-1-ioanna-maria.alifieraki@canonical.com
Fixes: a97955844807 ("ipc,sem: remove uneeded sem_undo_list lock usage in exit_sem()")
Signed-off-by: Ioanna Alifieraki <ioanna-maria.alifieraki@canonical.com>
Acked-by: Manfred Spraul <manfred@colorfullife.com>
Acked-by: Herton R. Krzesinski <herton@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: <malat@debian.org>
Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jay Vosburgh <jay.vosburgh@canonical.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent c766d14
History
File Mode Size
ABI
EDID
PCI
RCU
accounting
admin-guide
arm
arm64
block
bpf
cdrom
core-api
cpu-freq
crypto
dev-tools
devicetree
doc-guide
driver-api
fault-injection
fb
features
filesystems
firmware-guide
firmware_class
fpga
gpu
hid
hwmon
i2c
ia64
ide
iio
infiniband
input
isdn
kbuild
kernel-hacking
leds
livepatch
locking
m68k
maintainer
media
mic
mips
misc-devices
netlabel
networking
nios2
nvdimm
openrisc
parisc
pcmcia
power
powerpc
process
riscv
s390
scheduler
scsi
security
sh
sound
sparc
sphinx
sphinx-static
spi
target
timers
trace
translations
usb
userspace-api
virt
vm
w1
watchdog
x86
xtensa
.gitignore -rw-r--r-- 13 bytes
COPYING-logo -rw-r--r-- 563 bytes
Changes l--------- 19 bytes
CodingStyle -rw-r--r-- 48 bytes
DMA-API-HOWTO.txt -rw-r--r-- 32.8 KB
DMA-API.txt -rw-r--r-- 27.3 KB
DMA-ISA-LPC.txt -rw-r--r-- 5.1 KB
DMA-attributes.txt -rw-r--r-- 6.0 KB
IPMI.txt -rw-r--r-- 29.7 KB
IRQ-affinity.txt -rw-r--r-- 2.5 KB
IRQ-domain.txt -rw-r--r-- 10.9 KB
IRQ.txt -rw-r--r-- 994 bytes
Kconfig -rw-r--r-- 360 bytes
Makefile -rw-r--r-- 5.3 KB
SubmittingPatches -rw-r--r-- 54 bytes
asm-annotations.rst -rw-r--r-- 9.2 KB
atomic_bitops.txt -rw-r--r-- 1.5 KB
atomic_t.txt -rw-r--r-- 6.9 KB
bus-virt-phys-mapping.txt -rw-r--r-- 8.0 KB
conf.py -rw-r--r-- 20.4 KB
crc32.txt -rw-r--r-- 8.6 KB
debugging-modules.txt -rw-r--r-- 954 bytes
debugging-via-ohci1394.txt -rw-r--r-- 7.5 KB
digsig.txt -rw-r--r-- 3.0 KB
docutils.conf -rw-r--r-- 159 bytes
dontdiff -rw-r--r-- 2.6 KB
futex-requeue-pi.txt -rw-r--r-- 5.1 KB
hwspinlock.txt -rw-r--r-- 15.1 KB
index.rst -rw-r--r-- 4.0 KB
io-mapping.txt -rw-r--r-- 3.3 KB
io_ordering.txt -rw-r--r-- 2.0 KB
irqflags-tracing.txt -rw-r--r-- 2.3 KB
kobject.txt -rw-r--r-- 18.5 KB
kprobes.txt -rw-r--r-- 30.3 KB
kref.txt -rw-r--r-- 8.9 KB
logo.gif -rw-r--r-- 16.0 KB
lzo.txt -rw-r--r-- 9.1 KB
mailbox.txt -rw-r--r-- 4.4 KB
memory-barriers.txt -rw-r--r-- 114.4 KB
nommu-mmap.txt -rw-r--r-- 12.4 KB
percpu-rw-semaphore.txt -rw-r--r-- 1.1 KB
pi-futex.txt -rw-r--r-- 5.7 KB
preempt-locking.txt -rw-r--r-- 5.5 KB
rbtree.txt -rw-r--r-- 14.8 KB
remoteproc.txt -rw-r--r-- 12.8 KB
robust-futex-ABI.txt -rw-r--r-- 8.7 KB
robust-futexes.txt -rw-r--r-- 9.5 KB
rpmsg.txt -rw-r--r-- 13.1 KB
speculation.txt -rw-r--r-- 2.8 KB
static-keys.txt -rw-r--r-- 13.0 KB
tee.txt -rw-r--r-- 9.2 KB
this_cpu_ops.txt -rw-r--r-- 11.2 KB
unaligned-memory-access.txt -rw-r--r-- 10.4 KB
xz.txt -rw-r--r-- 5.5 KB

back to top