| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACT4Y+b8RN+y5zYUAXD_rd1VjFD8DPBwmcr1Ec7Ba8qY4d5j-w@mail.gmail.com>
Date: Tue, 21 Mar 2017 13:48:27 +0100
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Takashi Iwai <tiwai@...e.de>
Cc: alsa-devel@...a-project.org, Jaroslav Kysela <perex@...ex.cz>,
LKML <linux-kernel@...r.kernel.org>,
syzkaller <syzkaller@...glegroups.com>
Subject: Re: sound: another deadlock in snd_seq_pool_done
On Tue, Mar 21, 2017 at 1:23 PM, Takashi Iwai <tiwai@...e.de> wrote:
> On Sat, 04 Mar 2017 17:31:21 +0100,
> Dmitry Vyukov wrote:
>>
>> Hello,
>>
>> The following program creates processes deadlocked in snd_seq_pool_done:
>>
>> https://gist.githubusercontent.com/dvyukov/97efc9cb6d63b1b2c7b737b82cc8b0d6/raw/3546b133ae0b2d3e1190ae7c1f4e240ce7ce132e/gistfile1.txt
>>
>> After few seconds I get:
>>
>> # ps afxu | grep a.out
>> root 8660 2.0 0.0 0 0 pts/0 Zl 16:27 0:00
>> [a.out] <defunct>
>>
>> # kill -9 8660
>>
>> # cat /proc/8660/status
>> Name: a.out
>> State: Z (zombie)
>> Tgid: 8660
>> Ngid: 0
>> Pid: 8660
>> PPid: 1
>> TracerPid: 0
>> Uid: 0 0 0 0
>> Gid: 0 0 0 0
>> FDSize: 0
>> Groups: 0
>> NStgid: 8660
>> NSpid: 8660
>> NSpgid: 8660
>> NSsid: 2971
>> Threads: 2
>> SigQ: 1/3304
>> SigPnd: 0000000000000000
>> ShdPnd: 0000000000000100
>> SigBlk: 0000000000000000
>> SigIgn: 0000000180000000
>> SigCgt: 0000000000000440
>> CapInh: 0000000000000000
>> CapPrm: 0000003fffffffff
>> CapEff: 0000003fffffffff
>> CapBnd: 0000003fffffffff
>> CapAmb: 0000000000000000
>> NoNewPrivs: 0
>> Seccomp: 0
>> Cpus_allowed: f
>> Cpus_allowed_list: 0-3
>> Mems_allowed: 00000000,00000001
>> Mems_allowed_list: 0
>> voluntary_ctxt_switches: 12
>> nonvoluntary_ctxt_switches: 0
>>
>> # cat /proc/8660/task/*/stack
>> [<ffffffff835406db>] snd_seq_pool_done+0x31b/0x620
>> sound/core/seq/seq_memory.c:436
>> [<ffffffff8353a11e>] snd_seq_ioctl_set_client_pool+0x1ae/0x600
>> sound/core/seq/seq_clientmgr.c:1836
>> [<ffffffff835382ba>] snd_seq_ioctl+0x2da/0x4d0
>> sound/core/seq/seq_clientmgr.c:2130
>> [<ffffffff81aced2f>] vfs_ioctl fs/ioctl.c:45 [inline]
>> [<ffffffff81aced2f>] do_vfs_ioctl+0x1bf/0x1790 fs/ioctl.c:685
>> [<ffffffff81ad038f>] SYSC_ioctl fs/ioctl.c:700 [inline]
>> [<ffffffff81ad038f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
>> [<ffffffff8457dc41>] entry_SYSCALL_64_fastpath+0x1f/0xc2
>
> Now I've been trying to reproduce the hang, alas it didn't happen on
> my VM by some reason.
>
> In anyway, below is an untested fix for possible races against
> snd_seq_pool_done() and cell insertions. Could you check whether it
> cures your issue?
I reproduced it again within 10 second. Then restarted kernel with
this patch and now it runs for 10 minutes without any hanged
processes.
Tested-by: Dmitry Vyukov <dvyukov@...gle.com>
Thanks!
> ---
> diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
> index 4c935202ce23..f3b1d7f50b81 100644
> --- a/sound/core/seq/seq_clientmgr.c
> +++ b/sound/core/seq/seq_clientmgr.c
> @@ -1832,6 +1832,7 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,
> 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);
> }
> diff --git a/sound/core/seq/seq_fifo.c b/sound/core/seq/seq_fifo.c
> index 448efd4e980e..33980d1c8037 100644
> --- a/sound/core/seq/seq_fifo.c
> +++ b/sound/core/seq/seq_fifo.c
> @@ -72,6 +72,9 @@ void snd_seq_fifo_delete(struct snd_seq_fifo **fifo)
> return;
> *fifo = NULL;
>
> + if (f->pool)
> + snd_seq_pool_mark_closing(f->pool);
> +
> snd_seq_fifo_clear(f);
>
> /* wake up clients if any */
> diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c
> index 1a1acf3ddda4..d4c61ec9be13 100644
> --- a/sound/core/seq/seq_memory.c
> +++ b/sound/core/seq/seq_memory.c
> @@ -415,6 +415,18 @@ int snd_seq_pool_init(struct snd_seq_pool *pool)
> 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)
> {
> @@ -425,10 +437,6 @@ int snd_seq_pool_done(struct snd_seq_pool *pool)
> 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);
>
> @@ -485,6 +493,7 @@ int snd_seq_pool_delete(struct snd_seq_pool **ppool)
> *ppool = NULL;
> if (pool == NULL)
> return 0;
> + snd_seq_pool_mark_closing(pool);
> snd_seq_pool_done(pool);
> kfree(pool);
> return 0;
> diff --git a/sound/core/seq/seq_memory.h b/sound/core/seq/seq_memory.h
> index 4a2ec779b8a7..32f959c17786 100644
> --- 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(struct snd_seq_pool *pool)
> 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 */
Powered by blists - more mailing lists