From: Takashi Iwai <tiwai@suse.de>
To: "Dmitry Vyukov" <dvyukov@google.com>
Cc: <alsa-devel@alsa-project.org>, "Jaroslav Kysela" <perex@perex.cz>,
"LKML" <linux-kernel@vger.kernel.org>,
"Alexander Potapenko" <glider@google.com>,
"Kostya Serebryany" <kcc@google.com>,
"syzkaller" <syzkaller@googlegroups.com>,
"Sasha Levin" <sasha.levin@oracle.com>
Subject: Re: sound: memory leak in snd_seq_pool_init
Date: Mon, 15 Feb 2016 11:21:30 +0100 [thread overview]
Message-ID: <s5hziv2b7f9.wl-tiwai@suse.de> (raw)
In-Reply-To: <CACT4Y+bSzazpXNvtAr=WXaL8hptqjHwqEyFA+VN2AWEx=aurkg@mail.gmail.com>
On Mon, 15 Feb 2016 10:35:22 +0100,
Dmitry Vyukov wrote:
>
> Hello,
>
> The following program causes memory leak of a 24K object allocated in
> snd_seq_pool_init:
>
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <pthread.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
>
> long r[148];
>
> void* thr(void* arg)
> {
> switch ((long)arg) {
> case 0:
> r[0] = syscall(SYS_mmap, 0x20000000ul, 0xe000ul, 0x3ul, 0x32ul,
> 0xfffffffffffffffful, 0x0ul);
> break;
> case 1:
> r[2] = syscall(SYS_open, "/dev/snd/seq", 0x1ul, 0, 0, 0);
> break;
> case 2:
> *(uint32_t*)0x20008a90 = (uint32_t)0x6;
> *(uint32_t*)0x20008a94 = (uint32_t)0x4;
> *(uint32_t*)0x20008a98 = (uint32_t)0x200;
> *(uint8_t*)0x20008a9c = (uint8_t)0xfa5;
> *(uint8_t*)0x20008a9d = (uint8_t)0x100000000;
> *(uint8_t*)0x20008a9e = (uint8_t)0x0;
> *(uint8_t*)0x20008a9f = (uint8_t)0x3;
> *(uint8_t*)0x20008aa0 = (uint8_t)0x1;
> *(uint8_t*)0x20008aa1 = (uint8_t)0x1;
> *(uint8_t*)0x20008aa2 = (uint8_t)0x0;
> *(uint8_t*)0x20008aa3 = (uint8_t)0x40;
> *(uint8_t*)0x20008aa4 = (uint8_t)0x9;
> *(uint8_t*)0x20008aa5 = (uint8_t)0x9;
> *(uint8_t*)0x20008aa6 = (uint8_t)0x6;
> *(uint8_t*)0x20008aa7 = (uint8_t)0x80;
> *(uint8_t*)0x20008aa8 = (uint8_t)0x0;
> *(uint8_t*)0x20008aa9 = (uint8_t)0x5d34b2c3;
> *(uint8_t*)0x20008aaa = (uint8_t)0x3a7;
> *(uint8_t*)0x20008aab = (uint8_t)0x9;
> *(uint8_t*)0x20008aac = (uint8_t)0x8d62;
> *(uint8_t*)0x20008aad = (uint8_t)0x5;
> *(uint8_t*)0x20008aae = (uint8_t)0x7;
> *(uint8_t*)0x20008aaf = (uint8_t)0x20;
> *(uint8_t*)0x20008ab0 = (uint8_t)0x8;
> *(uint8_t*)0x20008ab1 = (uint8_t)0x7;
> *(uint8_t*)0x20008ab2 = (uint8_t)0xfffffffffffffffc;
> *(uint8_t*)0x20008ab3 = (uint8_t)0x7;
> *(uint8_t*)0x20008ab4 = (uint8_t)0x7ff;
> *(uint8_t*)0x20008ab5 = (uint8_t)0x1;
> *(uint8_t*)0x20008ab6 = (uint8_t)0x0;
> *(uint8_t*)0x20008ab7 = (uint8_t)0x9;
> *(uint8_t*)0x20008ab8 = (uint8_t)0x6;
> *(uint8_t*)0x20008ab9 = (uint8_t)0xfffffffffffffe00;
> *(uint8_t*)0x20008aba = (uint8_t)0x1000;
> *(uint8_t*)0x20008abb = (uint8_t)0xfffffffffffffffb;
> *(uint8_t*)0x20008abc = (uint8_t)0xcfd3;
> *(uint8_t*)0x20008abd = (uint8_t)0x101;
> *(uint8_t*)0x20008abe = (uint8_t)0x9;
> *(uint8_t*)0x20008abf = (uint8_t)0xfffffffffffffffc;
> *(uint8_t*)0x20008ac0 = (uint8_t)0xf2;
> *(uint8_t*)0x20008ac1 = (uint8_t)0x6;
> *(uint8_t*)0x20008ac2 = (uint8_t)0x0;
> *(uint8_t*)0x20008ac3 = (uint8_t)0x7fffffff;
> *(uint8_t*)0x20008ac4 = (uint8_t)0x4;
> *(uint8_t*)0x20008ac5 = (uint8_t)0x5;
> *(uint8_t*)0x20008ac6 = (uint8_t)0x4;
> *(uint8_t*)0x20008ac7 = (uint8_t)0x4;
> *(uint8_t*)0x20008ac8 = (uint8_t)0x20;
> *(uint8_t*)0x20008ac9 = (uint8_t)0x401;
> *(uint8_t*)0x20008aca = (uint8_t)0x6;
> *(uint8_t*)0x20008acb = (uint8_t)0x6;
> *(uint8_t*)0x20008acc = (uint8_t)0x5;
> *(uint8_t*)0x20008acd = (uint8_t)0x3;
> *(uint8_t*)0x20008ace = (uint8_t)0xfffffffffffffffc;
> *(uint8_t*)0x20008acf = (uint8_t)0xb0c;
> *(uint8_t*)0x20008ad0 = (uint8_t)0x2;
> *(uint8_t*)0x20008ad1 = (uint8_t)0xfffffffffffffff9;
> *(uint8_t*)0x20008ad2 = (uint8_t)0x8000;
> *(uint8_t*)0x20008ad3 = (uint8_t)0x5;
> *(uint8_t*)0x20008ad4 = (uint8_t)0xb0c;
> *(uint8_t*)0x20008ad5 = (uint8_t)0xfffffffffffffeff;
> *(uint8_t*)0x20008ad6 = (uint8_t)0x5;
> *(uint8_t*)0x20008ad7 = (uint8_t)0x101;
> *(uint8_t*)0x20008ad8 = (uint8_t)0xbfd;
> *(uint8_t*)0x20008ad9 = (uint8_t)0x7ff;
> *(uint8_t*)0x20008ada = (uint8_t)0x7f;
> *(uint8_t*)0x20008adb = (uint8_t)0x5;
> *(uint32_t*)0x20008adc = (uint32_t)0x6;
> *(uint8_t*)0x20008ae0 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae1 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae2 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae3 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae4 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae5 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae6 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae7 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae8 = (uint8_t)0x0;
> *(uint8_t*)0x20008ae9 = (uint8_t)0x0;
> *(uint8_t*)0x20008aea = (uint8_t)0x0;
> *(uint8_t*)0x20008aeb = (uint8_t)0x0;
> *(uint8_t*)0x20008aec = (uint8_t)0x0;
> *(uint8_t*)0x20008aed = (uint8_t)0x0;
> *(uint8_t*)0x20008aee = (uint8_t)0x0;
> *(uint8_t*)0x20008aef = (uint8_t)0x0;
> *(uint8_t*)0x20008af0 = (uint8_t)0x0;
> *(uint8_t*)0x20008af1 = (uint8_t)0x0;
> *(uint8_t*)0x20008af2 = (uint8_t)0x0;
> *(uint8_t*)0x20008af3 = (uint8_t)0x0;
> *(uint8_t*)0x20008af4 = (uint8_t)0x0;
> *(uint8_t*)0x20008af5 = (uint8_t)0x0;
> *(uint8_t*)0x20008af6 = (uint8_t)0x0;
> *(uint8_t*)0x20008af7 = (uint8_t)0x0;
> *(uint8_t*)0x20008af8 = (uint8_t)0x0;
> *(uint8_t*)0x20008af9 = (uint8_t)0x0;
> *(uint8_t*)0x20008afa = (uint8_t)0x0;
> *(uint8_t*)0x20008afb = (uint8_t)0x0;
> *(uint8_t*)0x20008afc = (uint8_t)0x0;
> *(uint8_t*)0x20008afd = (uint8_t)0x0;
> *(uint8_t*)0x20008afe = (uint8_t)0x0;
> *(uint8_t*)0x20008aff = (uint8_t)0x0;
> *(uint8_t*)0x20008b00 = (uint8_t)0x0;
> *(uint8_t*)0x20008b01 = (uint8_t)0x0;
> *(uint8_t*)0x20008b02 = (uint8_t)0x0;
> *(uint8_t*)0x20008b03 = (uint8_t)0x0;
> *(uint8_t*)0x20008b04 = (uint8_t)0x0;
> *(uint8_t*)0x20008b05 = (uint8_t)0x0;
> *(uint8_t*)0x20008b06 = (uint8_t)0x0;
> *(uint8_t*)0x20008b07 = (uint8_t)0x0;
> *(uint8_t*)0x20008b08 = (uint8_t)0x0;
> *(uint8_t*)0x20008b09 = (uint8_t)0x0;
> *(uint8_t*)0x20008b0a = (uint8_t)0x0;
> *(uint8_t*)0x20008b0b = (uint8_t)0x0;
> *(uint8_t*)0x20008b0c = (uint8_t)0x0;
> *(uint8_t*)0x20008b0d = (uint8_t)0x0;
> *(uint8_t*)0x20008b0e = (uint8_t)0x0;
> *(uint8_t*)0x20008b0f = (uint8_t)0x0;
> *(uint8_t*)0x20008b10 = (uint8_t)0x0;
> *(uint8_t*)0x20008b11 = (uint8_t)0x0;
> *(uint8_t*)0x20008b12 = (uint8_t)0x0;
> *(uint8_t*)0x20008b13 = (uint8_t)0x0;
> *(uint8_t*)0x20008b14 = (uint8_t)0x0;
> *(uint8_t*)0x20008b15 = (uint8_t)0x0;
> *(uint8_t*)0x20008b16 = (uint8_t)0x0;
> *(uint8_t*)0x20008b17 = (uint8_t)0x0;
> *(uint8_t*)0x20008b18 = (uint8_t)0x0;
> *(uint8_t*)0x20008b19 = (uint8_t)0x0;
> *(uint8_t*)0x20008b1a = (uint8_t)0x0;
> *(uint8_t*)0x20008b1b = (uint8_t)0x0;
> r[131] =
> syscall(SYS_ioctl, r[2], 0xc08c5332ul, 0x20008a90ul, 0, 0, 0);
> break;
> case 3:
> *(uint32_t*)0x20009fd0 = (uint32_t)0x30;
> *(uint32_t*)0x20009fd4 = (uint32_t)0xfff;
> *(uint64_t*)0x20009fd8 = (uint64_t)0x0;
> *(uint64_t*)0x20009fe0 = (uint64_t)0x1ce;
> *(uint64_t*)0x20009fe8 = (uint64_t)0x2;
> *(uint64_t*)0x20009ff0 = (uint64_t)0xff;
> *(uint32_t*)0x20009ff8 = (uint32_t)0x10001;
> r[139] = syscall(SYS_write, r[2], 0x20009fd0ul, 0x30ul, 0, 0, 0);
> break;
> case 4:
> *(uint32_t*)0x2000bb13 = (uint32_t)0x30;
> *(uint32_t*)0x2000bb17 = (uint32_t)0x2;
> *(uint64_t*)0x2000bb1b = (uint64_t)0x0;
> *(uint64_t*)0x2000bb23 = (uint64_t)0x53;
> *(uint64_t*)0x2000bb2b = (uint64_t)0xfffffffffffffbff;
> *(uint64_t*)0x2000bb33 = (uint64_t)0x5;
> *(uint32_t*)0x2000bb3b = (uint32_t)0x5;
> r[147] = syscall(SYS_write, r[2], 0x2000bb13ul, 0x30ul, 0, 0, 0);
> break;
> }
> return 0;
> }
>
> int main()
> {
> long i;
> pthread_t th[5];
>
> memset(r, -1, sizeof(r));
> for (i = 0; i < 5; i++) {
> pthread_create(&th[i], 0, thr, (void*)i);
> usleep(10000);
> }
> for (i = 0; i < 5; i++) {
> pthread_create(&th[i], 0, thr, (void*)i);
> if (i % 2 == 0)
> usleep(10000);
> }
> usleep(100000);
> return 0;
> }
>
>
>
> unreferenced object 0xffffc90003663000 (size 24000):
> comm "a.out", pid 1770, jiffies 4295717383 (age 7.217s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff86644303>] kmemleak_alloc+0x63/0xa0 mm/kmemleak.c:916
> [<ffffffff81720da9>] __vmalloc_node_range+0x469/0x640 mm/vmalloc.c:1682
> [< inline >] __vmalloc_node mm/vmalloc.c:1710
> [< inline >] __vmalloc_node_flags mm/vmalloc.c:1724
> [<ffffffff8172104b>] vmalloc+0x5b/0x70 mm/vmalloc.c:1739
> [<ffffffff852b92cb>] snd_seq_pool_init+0xbb/0x300
> sound/core/seq/seq_memory.c:389
> [<ffffffff852b5ff4>] snd_seq_write+0x4d4/0x570
> sound/core/seq/seq_clientmgr.c:1029
> [<ffffffff817bbfc3>] __vfs_write+0x113/0x4b0 fs/read_write.c:528
> [<ffffffff817bda87>] vfs_write+0x167/0x4a0 fs/read_write.c:577
> [< inline >] SYSC_write fs/read_write.c:624
> [<ffffffff817c0d71>] SyS_write+0x111/0x220 fs/read_write.c:616
> [<ffffffff866613f6>] entry_SYSCALL_64_fastpath+0x16/0x7a
> arch/x86/entry/entry_64.S:185
> [<ffffffffffffffff>] 0xffffffffffffffff
>
>
> On commit 388f7b1d6e8ca06762e2454d28d6c3c55ad0fe95 (4.5-rc3)
Hm, this looks like the issue triggered by concurrent writes to the
same client. Does the patch fix the problem?
thanks,
Takashi
---
diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c
index 801076687bb1..c850345c43b5 100644
--- a/sound/core/seq/seq_memory.c
+++ b/sound/core/seq/seq_memory.c
@@ -383,15 +383,20 @@ int snd_seq_pool_init(struct snd_seq_pool *pool)
if (snd_BUG_ON(!pool))
return -EINVAL;
- if (pool->ptr) /* should be atomic? */
- return 0;
- pool->ptr = vmalloc(sizeof(struct snd_seq_event_cell) * pool->size);
- if (!pool->ptr)
+ cellptr = vmalloc(sizeof(struct snd_seq_event_cell) * pool->size);
+ if (!cellptr)
return -ENOMEM;
/* add new cells to the free cell list */
spin_lock_irqsave(&pool->lock, flags);
+ if (pool->ptr) {
+ spin_unlock_irqrestore(&pool->lock, flags);
+ vfree(cellptr);
+ return 0;
+ }
+
+ pool->ptr = cellptr;
pool->free = NULL;
for (cell = 0; cell < pool->size; cell++) {
next prev parent reply other threads:[~2016-02-15 10:21 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-15 9:35 Dmitry Vyukov
2016-02-15 10:21 ` Takashi Iwai [this message]
2016-02-15 15:15 ` Dmitry Vyukov
2016-02-15 15:30 ` Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5hziv2b7f9.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=alsa-devel@alsa-project.org \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=kcc@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=sasha.levin@oracle.com \
--cc=syzkaller@googlegroups.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®