Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1688380

[PATCH 3.16 072/178] ALSA: seq: Fix racy cell insertions during snd_seq_pool_done()

From Ben Hutchings <ben@decadent.org.uk>
Newsgroups linux.kernel
Subject [PATCH 3.16 072/178] ALSA: seq: Fix racy cell insertions during snd_seq_pool_done()
Date 2017-07-16 16:40 +0200
Message-ID <u3SSi-fg-77@gated-at.bofh.it> (permalink)
References <u3Sfv-89R-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw

3.16.46-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Takashi Iwai <tiwai@suse.de>

commit c520ff3d03f0b5db7146d9beed6373ad5d2a5e0e upstream.

When snd_seq_pool_done() is called, it marks the closing flag to
refuse the further cell insertions.  But snd_seq_pool_done() itself
doesn't clear the cells but just waits until all cells are cleared by
the caller side.  That is, it's racy, and this leads to the endless
stall as syzkaller spotted.

This patch addresses the racy by splitting the setup of pool->closing
flag out of snd_seq_pool_done(), and calling it properly before
snd_seq_pool_done().

BugLink: http://lkml.kernel.org/r/CACT4Y+aqqy8bZA1fFieifNxR2fAfFQQABcBHj801+u5ePV0URw@mail.gmail.com
Reported-and-tested-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 sound/core/seq/seq_clientmgr.c |  1 +
 sound/core/seq/seq_fifo.c      |  3 +++
 sound/core/seq/seq_memory.c    | 17 +++++++++++++----
 sound/core/seq/seq_memory.h    |  1 +
 4 files changed, 18 insertions(+), 4 deletions(-)

--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1921,6 +1921,7 @@ static int snd_seq_ioctl_set_client_pool
 	     info.output_pool != client->pool->size)) {
 		if (snd_seq_write_pool_allocated(client)) {
 			/* remove all existing cells */
+			snd_seq_pool_mark_closing(client->pool);
 			snd_seq_queue_client_leave_cells(client->number);
 			snd_seq_pool_done(client->pool);
 		}
--- a/sound/core/seq/seq_fifo.c
+++ b/sound/core/seq/seq_fifo.c
@@ -70,6 +70,9 @@ void snd_seq_fifo_delete(struct snd_seq_
 		return;
 	*fifo = NULL;
 
+	if (f->pool)
+		snd_seq_pool_mark_closing(f->pool);
+
 	snd_seq_fifo_clear(f);
 
 	/* wake up clients if any */
--- a/sound/core/seq/seq_memory.c
+++ b/sound/core/seq/seq_memory.c
@@ -414,6 +414,18 @@ int snd_seq_pool_init(struct snd_seq_poo
 	return 0;
 }
 
+/* refuse the further insertion to the pool */
+void snd_seq_pool_mark_closing(struct snd_seq_pool *pool)
+{
+	unsigned long flags;
+
+	if (snd_BUG_ON(!pool))
+		return;
+	spin_lock_irqsave(&pool->lock, flags);
+	pool->closing = 1;
+	spin_unlock_irqrestore(&pool->lock, flags);
+}
+
 /* remove events */
 int snd_seq_pool_done(struct snd_seq_pool *pool)
 {
@@ -424,10 +436,6 @@ int snd_seq_pool_done(struct snd_seq_poo
 		return -EINVAL;
 
 	/* wait for closing all threads */
-	spin_lock_irqsave(&pool->lock, flags);
-	pool->closing = 1;
-	spin_unlock_irqrestore(&pool->lock, flags);
-
 	if (waitqueue_active(&pool->output_sleep))
 		wake_up(&pool->output_sleep);
 
@@ -484,6 +492,7 @@ int snd_seq_pool_delete(struct snd_seq_p
 	*ppool = NULL;
 	if (pool == NULL)
 		return 0;
+	snd_seq_pool_mark_closing(pool);
 	snd_seq_pool_done(pool);
 	kfree(pool);
 	return 0;
--- a/sound/core/seq/seq_memory.h
+++ b/sound/core/seq/seq_memory.h
@@ -84,6 +84,7 @@ static inline int snd_seq_total_cells(st
 int snd_seq_pool_init(struct snd_seq_pool *pool);
 
 /* done pool - free events */
+void snd_seq_pool_mark_closing(struct snd_seq_pool *pool);
 int snd_seq_pool_done(struct snd_seq_pool *pool);
 
 /* create pool */

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 3.16 000/178] 3.16.46-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 077/178] s390/decompressor: fix initrd corruption  caused by bss clear Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 132/178] metag/usercopy: Set flags before ADDZ Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 024/178] [media] dvb-usb-firmware: don't do DMA on stack Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 144/178] x86/vdso: Ensure vdso32_enabled gets set to  valid values only Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 168/178] macvlan: Fix device ref leak when purging  bc_queue Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 048/178] futex: Add missing error handling to  FUTEX_REQUEUE_PI Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 123/178] l2tp: take reference on sessions being dumped Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 056/178] Input: iforce - validate number of endpoints  before using them Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 073/178] net: unix: properly re-increment inflight  counter of GC discarded candidates Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  Re: [PATCH 3.16 000/178] 3.16.46-rc1 review Guenter Roeck <linux@roeck-us.net> - 2017-07-16 16:40 +0200
    Re: [PATCH 3.16 000/178] 3.16.46-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 18:40 +0200
  [PATCH 3.16 007/178] batman-adv: Keep fragments equally sized Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 107/178] drm/vmwgfx: Remove getparam error message Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 072/178] ALSA: seq: Fix racy cell insertions during  snd_seq_pool_done() Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 055/178] mmc: ushc: fix NULL-deref at probe Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 136/178] Reset TreeId to zero on SMB2 TREE_CONNECT Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 175/178] char: lp: fix possible integer overflow in  lp_setup() Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:40 +0200
  [PATCH 3.16 044/178] usb-core: Add LINEAR_FRAME_INTR_BINTERVAL  USB quirk Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 070/178] ALSA: ctxfi: Fix the incorrect check of  dma_set_mask() call Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 087/178] ALSA: seq: Fix race during FIFO resize Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 086/178] hwmon: (asus_atk0110) fix uninitialized data  access Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 066/178] usb: hub: Fix crash after failure to read  BOS descriptor Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 166/178] net: ipv6: send unsolicited NA if enabled  for all interfaces Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 006/178] tracing: Add #undef to fix compile error Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 004/178] kprobes/x86: Fix kernel panic when certain  exception-handling addresses are probed Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 170/178] ipv6: move stub initialization after ipv6  setup completion Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 025/178] USB: iowarrior: fix NULL-deref in write Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 150/178] cpupower: Fix turbo frequency reporting for  pre-Sandy Bridge cores Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 176/178] fs/exec.c: account for argv/envp pointers Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 127/178] ring-buffer: Fix return value check in  test_ringbuffer() Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 015/178] Input: i8042 - add noloop quirk for Dell  Embedded Box PC 3000 Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 167/178] Input: i8042 - add Clevo P650RS to the i8042  reset list Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 050/178] iio: adc: ti_am335x_adc: fix fifo overrun  recovery Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 049/178] ext4: mark inode dirty after converting  inline directory Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200
  [PATCH 3.16 116/178] include/linux/kernel.h: change abs() macro  so it uses consistent return type Ben Hutchings <ben@decadent.org.uk> - 2017-07-16 16:50 +0200

csiph-web