* tty: tty_struct memory leak
@ 2016-02-03 16:10 Dmitry Vyukov
2016-02-03 16:26 ` Dmitry Vyukov
2016-02-05 18:49 ` [PATCH] tty: Drop krefs for interrupted tty lock Peter Hurley
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2016-02-03 16:10 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, LKML
Hello,
The following program causes tty_struct memory leak:
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <pthread.h>
#include <stdint.h>
#include <string.h>
#include <sys/syscall.h>
#include <unistd.h>
int main()
{
alarm(1);
syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0);
return 0;
}
unreferenced object 0xffff88002d3c5898 (size 2048):
comm "a.out", pid 5831, jiffies 4303981829 (age 9.451s)
hex dump (first 32 bytes):
01 54 00 00 1c 00 00 00 98 58 14 63 00 88 ff ff .T.......X.c....
18 ec 12 63 00 88 ff ff c0 45 34 87 ff ff ff ff ...c.....E4.....
backtrace:
[< inline >] kzalloc include/linux/slab.h:607
[<ffffffff82f871c8>] alloc_tty_struct+0x98/0x820 drivers/tty/tty_io.c:3133
[<ffffffff82f879c8>] tty_init_dev+0x78/0x4b0 drivers/tty/tty_io.c:1523
[<ffffffff82f88abd>] tty_open+0xcbd/0x1070 drivers/tty/tty_io.c:2082
[<ffffffff817c864a>] chrdev_open+0x22a/0x4c0 fs/char_dev.c:388
[<ffffffff817b3e72>] do_dentry_open+0x6a2/0xcb0 fs/open.c:736
[<ffffffff817b754b>] vfs_open+0x17b/0x1f0 fs/open.c:853
[< inline >] do_last fs/namei.c:3254
[<ffffffff817ead19>] path_openat+0xde9/0x5e30 fs/namei.c:3386
[<ffffffff817f359e>] do_filp_open+0x18e/0x250 fs/namei.c:3421
[<ffffffff817b7ccc>] do_sys_open+0x1fc/0x420 fs/open.c:1022
[< inline >] SYSC_open fs/open.c:1040
[<ffffffff817b7f1d>] SyS_open+0x2d/0x40 fs/open.c:1035
[<ffffffff8665ebb6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
# ls -l /dev/ircomm7
crw-rw---T 1 root dialout 161, 7 Feb 3 16:03 /dev/ircomm7
On commit 34229b277480f46c1e9a19f027f30b074512e68b
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: tty: tty_struct memory leak 2016-02-03 16:10 tty: tty_struct memory leak Dmitry Vyukov @ 2016-02-03 16:26 ` Dmitry Vyukov 2016-02-03 23:27 ` Peter Hurley 2016-02-05 18:49 ` [PATCH] tty: Drop krefs for interrupted tty lock Peter Hurley 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Vyukov @ 2016-02-03 16:26 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby, LKML Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > Hello, > > The following program causes tty_struct memory leak: > > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include <pthread.h> > #include <stdint.h> > #include <string.h> > #include <sys/syscall.h> > #include <unistd.h> > > int main() > { > alarm(1); > syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); > return 0; > } > > > unreferenced object 0xffff88002d3c5898 (size 2048): > comm "a.out", pid 5831, jiffies 4303981829 (age 9.451s) > hex dump (first 32 bytes): > 01 54 00 00 1c 00 00 00 98 58 14 63 00 88 ff ff .T.......X.c.... > 18 ec 12 63 00 88 ff ff c0 45 34 87 ff ff ff ff ...c.....E4..... > backtrace: > [< inline >] kzalloc include/linux/slab.h:607 > [<ffffffff82f871c8>] alloc_tty_struct+0x98/0x820 drivers/tty/tty_io.c:3133 > [<ffffffff82f879c8>] tty_init_dev+0x78/0x4b0 drivers/tty/tty_io.c:1523 > [<ffffffff82f88abd>] tty_open+0xcbd/0x1070 drivers/tty/tty_io.c:2082 > [<ffffffff817c864a>] chrdev_open+0x22a/0x4c0 fs/char_dev.c:388 > [<ffffffff817b3e72>] do_dentry_open+0x6a2/0xcb0 fs/open.c:736 > [<ffffffff817b754b>] vfs_open+0x17b/0x1f0 fs/open.c:853 > [< inline >] do_last fs/namei.c:3254 > [<ffffffff817ead19>] path_openat+0xde9/0x5e30 fs/namei.c:3386 > [<ffffffff817f359e>] do_filp_open+0x18e/0x250 fs/namei.c:3421 > [<ffffffff817b7ccc>] do_sys_open+0x1fc/0x420 fs/open.c:1022 > [< inline >] SYSC_open fs/open.c:1040 > [<ffffffff817b7f1d>] SyS_open+0x2d/0x40 fs/open.c:1035 > [<ffffffff8665ebb6>] entry_SYSCALL_64_fastpath+0x16/0x7a > arch/x86/entry/entry_64.S:185 > > > # ls -l /dev/ircomm7 > crw-rw---T 1 root dialout 161, 7 Feb 3 16:03 /dev/ircomm7 > > > On commit 34229b277480f46c1e9a19f027f30b074512e68b +syzkaller mailing list ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tty: tty_struct memory leak 2016-02-03 16:26 ` Dmitry Vyukov @ 2016-02-03 23:27 ` Peter Hurley 2016-02-04 10:48 ` Dmitry Vyukov 0 siblings, 1 reply; 6+ messages in thread From: Peter Hurley @ 2016-02-03 23:27 UTC (permalink / raw) To: Dmitry Vyukov, Greg Kroah-Hartman, Jiri Slaby, LKML Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin Hi Dmitry, On 02/03/2016 08:26 AM, Dmitry Vyukov wrote: > On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote: >> Hello, >> >> The following program causes tty_struct memory leak: >> >> // autogenerated by syzkaller (http://github.com/google/syzkaller) >> #include <pthread.h> >> #include <stdint.h> >> #include <string.h> >> #include <sys/syscall.h> >> #include <unistd.h> >> >> int main() >> { >> alarm(1); >> syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); >> return 0; >> } Going to need more information than this because the reproducer above does not generate a tty_struct memory leak. Here's what I did: Enabled tty debugging and added patch below [1] to show kfree(tty), then: $ sudo modprobe ircomm $ ./reproducer Here's what I got: [ 1436.864342] tty_ldisc_open: ircomm ircomm7: ffff8802aa3b3410: opened [ 1436.864352] tty_open: ircomm ircomm7: opening (count=1) [ 1437.863994] tty_open: ircomm ircomm7: open error -512, releasing [ 1437.864051] tty_release: ircomm ircomm7: releasing (count=1) [ 1437.864055] tty_wait_until_sent: ircomm ircomm7: wait until sent, timeout=7500 [ 1437.864110] tty_release: ircomm ircomm7: final close [ 1437.864120] tty_ldisc_close: ircomm ircomm7: ffff8802aa3b3410: closed [ 1437.864124] tty_ldisc_release: ircomm ircomm7: released [ 1437.864130] tty_release: ircomm ircomm7: release [ 1437.864148] release_one_tty: ircomm ircomm7: freeing structure ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Note that release_one_tty() ends in kfree(tty) Regards, Peter Hurley >> unreferenced object 0xffff88002d3c5898 (size 2048): >> comm "a.out", pid 5831, jiffies 4303981829 (age 9.451s) >> hex dump (first 32 bytes): >> 01 54 00 00 1c 00 00 00 98 58 14 63 00 88 ff ff .T.......X.c.... >> 18 ec 12 63 00 88 ff ff c0 45 34 87 ff ff ff ff ...c.....E4..... >> backtrace: >> [< inline >] kzalloc include/linux/slab.h:607 >> [<ffffffff82f871c8>] alloc_tty_struct+0x98/0x820 drivers/tty/tty_io.c:3133 >> [<ffffffff82f879c8>] tty_init_dev+0x78/0x4b0 drivers/tty/tty_io.c:1523 >> [<ffffffff82f88abd>] tty_open+0xcbd/0x1070 drivers/tty/tty_io.c:2082 >> [<ffffffff817c864a>] chrdev_open+0x22a/0x4c0 fs/char_dev.c:388 >> [<ffffffff817b3e72>] do_dentry_open+0x6a2/0xcb0 fs/open.c:736 >> [<ffffffff817b754b>] vfs_open+0x17b/0x1f0 fs/open.c:853 >> [< inline >] do_last fs/namei.c:3254 >> [<ffffffff817ead19>] path_openat+0xde9/0x5e30 fs/namei.c:3386 >> [<ffffffff817f359e>] do_filp_open+0x18e/0x250 fs/namei.c:3421 >> [<ffffffff817b7ccc>] do_sys_open+0x1fc/0x420 fs/open.c:1022 >> [< inline >] SYSC_open fs/open.c:1040 >> [<ffffffff817b7f1d>] SyS_open+0x2d/0x40 fs/open.c:1035 >> [<ffffffff8665ebb6>] entry_SYSCALL_64_fastpath+0x16/0x7a >> arch/x86/entry/entry_64.S:185 >> >> >> # ls -l /dev/ircomm7 >> crw-rw---T 1 root dialout 161, 7 Feb 3 16:03 /dev/ircomm7 >> >> >> On commit 34229b277480f46c1e9a19f027f30b074512e68b > > +syzkaller mailing list > [1] --- >% --- Subject: [PATCH] debug: log tty freed --- drivers/tty/tty_io.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 3f4f47a..15f2d6d 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1633,6 +1633,8 @@ static void release_one_tty(struct work_struct *work) struct tty_driver *driver = tty->driver; struct module *owner = driver->owner; + tty_debug_hangup(tty, "freeing structure\n"); + if (tty->ops->cleanup) tty->ops->cleanup(tty); @@ -1909,7 +1911,7 @@ int tty_release(struct inode *inode, struct file *filp) /* Wait for pending work before tty destruction commmences */ tty_flush_works(tty); - tty_debug_hangup(tty, "freeing structure\n"); + tty_debug_hangup(tty, "release\n"); /* * The release_tty function takes care of the details of clearing * the slots and preserving the termios structure. The tty_unlock_pair -- 2.7.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: tty: tty_struct memory leak 2016-02-03 23:27 ` Peter Hurley @ 2016-02-04 10:48 ` Dmitry Vyukov 2016-02-04 21:48 ` Peter Hurley 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Vyukov @ 2016-02-04 10:48 UTC (permalink / raw) To: Peter Hurley Cc: Greg Kroah-Hartman, Jiri Slaby, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On Thu, Feb 4, 2016 at 12:27 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > Hi Dmitry, > > On 02/03/2016 08:26 AM, Dmitry Vyukov wrote: >> On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote: >>> Hello, >>> >>> The following program causes tty_struct memory leak: >>> >>> // autogenerated by syzkaller (http://github.com/google/syzkaller) >>> #include <pthread.h> >>> #include <stdint.h> >>> #include <string.h> >>> #include <sys/syscall.h> >>> #include <unistd.h> >>> >>> int main() >>> { >>> alarm(1); >>> syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); >>> return 0; >>> } > > Going to need more information than this because the reproducer > above does not generate a tty_struct memory leak. > > Here's what I did: > > Enabled tty debugging and added patch below [1] to show kfree(tty), then: > > $ sudo modprobe ircomm > $ ./reproducer > > Here's what I got: > > [ 1436.864342] tty_ldisc_open: ircomm ircomm7: ffff8802aa3b3410: opened > [ 1436.864352] tty_open: ircomm ircomm7: opening (count=1) > [ 1437.863994] tty_open: ircomm ircomm7: open error -512, releasing > [ 1437.864051] tty_release: ircomm ircomm7: releasing (count=1) > [ 1437.864055] tty_wait_until_sent: ircomm ircomm7: wait until sent, timeout=7500 > [ 1437.864110] tty_release: ircomm ircomm7: final close > [ 1437.864120] tty_ldisc_close: ircomm ircomm7: ffff8802aa3b3410: closed > [ 1437.864124] tty_ldisc_release: ircomm ircomm7: released > [ 1437.864130] tty_release: ircomm ircomm7: release > [ 1437.864148] release_one_tty: ircomm ircomm7: freeing structure > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > Note that release_one_tty() ends in kfree(tty) There seems to be some race, please try this one: // autogenerated by syzkaller (http://github.com/google/syzkaller) #include <pthread.h> #include <stdint.h> #include <stdlib.h> #include <string.h> #include <sys/syscall.h> #include <unistd.h> #include <sys/types.h> #include <sys/wait.h> void work() { alarm(1); syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); } int main() { int running, status; for (;;) { while (running < 32) { if (fork() == 0) { work(); exit(0); } running++; } if (wait(&status) > 0) running--; } } If I sample /proc/slabinfo while it runs: # cat /proc/slabinfo | egrep "^kmalloc-2048" Number of allocated objects constantly grow. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: tty: tty_struct memory leak 2016-02-04 10:48 ` Dmitry Vyukov @ 2016-02-04 21:48 ` Peter Hurley 0 siblings, 0 replies; 6+ messages in thread From: Peter Hurley @ 2016-02-04 21:48 UTC (permalink / raw) To: Dmitry Vyukov Cc: Greg Kroah-Hartman, Jiri Slaby, LKML, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin On 02/04/2016 02:48 AM, Dmitry Vyukov wrote: > On Thu, Feb 4, 2016 at 12:27 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >> Hi Dmitry, >> >> On 02/03/2016 08:26 AM, Dmitry Vyukov wrote: >>> On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvyukov@google.com> wrote: >>>> Hello, >>>> >>>> The following program causes tty_struct memory leak: >>>> >>>> // autogenerated by syzkaller (http://github.com/google/syzkaller) >>>> #include <pthread.h> >>>> #include <stdint.h> >>>> #include <string.h> >>>> #include <sys/syscall.h> >>>> #include <unistd.h> >>>> >>>> int main() >>>> { >>>> alarm(1); >>>> syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); >>>> return 0; >>>> } >> >> Going to need more information than this because the reproducer >> above does not generate a tty_struct memory leak. >> >> Here's what I did: >> >> Enabled tty debugging and added patch below [1] to show kfree(tty), then: >> >> $ sudo modprobe ircomm >> $ ./reproducer >> >> Here's what I got: >> >> [ 1436.864342] tty_ldisc_open: ircomm ircomm7: ffff8802aa3b3410: opened >> [ 1436.864352] tty_open: ircomm ircomm7: opening (count=1) >> [ 1437.863994] tty_open: ircomm ircomm7: open error -512, releasing >> [ 1437.864051] tty_release: ircomm ircomm7: releasing (count=1) >> [ 1437.864055] tty_wait_until_sent: ircomm ircomm7: wait until sent, timeout=7500 >> [ 1437.864110] tty_release: ircomm ircomm7: final close >> [ 1437.864120] tty_ldisc_close: ircomm ircomm7: ffff8802aa3b3410: closed >> [ 1437.864124] tty_ldisc_release: ircomm ircomm7: released >> [ 1437.864130] tty_release: ircomm ircomm7: release >> [ 1437.864148] release_one_tty: ircomm ircomm7: freeing structure >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ >> >> Note that release_one_tty() ends in kfree(tty) > > > There seems to be some race, please try this one: Yes, I see the bug now, thanks. > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include <pthread.h> > #include <stdint.h> > #include <stdlib.h> > #include <string.h> > #include <sys/syscall.h> > #include <unistd.h> > #include <sys/types.h> > #include <sys/wait.h> > > void work() > { > alarm(1); > syscall(SYS_open, "/dev/ircomm7", 0x12d401ul, 0, 0, 0); > } > > int main() { > int running, status; > > for (;;) { > while (running < 32) { > if (fork() == 0) { > work(); > exit(0); > } > running++; > } > if (wait(&status) > 0) > running--; > } > } > > > If I sample /proc/slabinfo while it runs: > > # cat /proc/slabinfo | egrep "^kmalloc-2048" > > Number of allocated objects constantly grow. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] tty: Drop krefs for interrupted tty lock 2016-02-03 16:10 tty: tty_struct memory leak Dmitry Vyukov 2016-02-03 16:26 ` Dmitry Vyukov @ 2016-02-05 18:49 ` Peter Hurley 1 sibling, 0 replies; 6+ messages in thread From: Peter Hurley @ 2016-02-05 18:49 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, Dmitry Vyukov, Peter Hurley When the tty lock is interrupted on attempted re-open, 2 tty krefs are still held. Drop extra kref before returning failure from tty_lock_interruptible(), and drop lookup kref before returning failure from tty_open(). Fixes: 0bfd464d3fdd ("tty: Wait interruptibly for tty lock on reopen") Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Peter Hurley <peter@hurleysoftware.com> --- Greg, This patch applies to tty-linus. For tty-next, the first blob has been refactored into tty_open_by_driver(). Please let me know if the merge isn't as straightforward as I believe it should be. drivers/tty/tty_io.c | 3 +-- drivers/tty/tty_mutex.c | 7 ++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 5cec01c..a7eacef 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2066,13 +2066,12 @@ retry_open: if (tty) { mutex_unlock(&tty_mutex); retval = tty_lock_interruptible(tty); + tty_kref_put(tty); /* drop kref from tty_driver_lookup_tty() */ if (retval) { if (retval == -EINTR) retval = -ERESTARTSYS; goto err_unref; } - /* safe to drop the kref from tty_driver_lookup_tty() */ - tty_kref_put(tty); retval = tty_reopen(tty); if (retval < 0) { tty_unlock(tty); diff --git a/drivers/tty/tty_mutex.c b/drivers/tty/tty_mutex.c index d2f3c4c..dfa9ec0 100644 --- a/drivers/tty/tty_mutex.c +++ b/drivers/tty/tty_mutex.c @@ -21,10 +21,15 @@ EXPORT_SYMBOL(tty_lock); int tty_lock_interruptible(struct tty_struct *tty) { + int ret; + if (WARN(tty->magic != TTY_MAGIC, "L Bad %p\n", tty)) return -EIO; tty_kref_get(tty); - return mutex_lock_interruptible(&tty->legacy_mutex); + ret = mutex_lock_interruptible(&tty->legacy_mutex); + if (ret) + tty_kref_put(tty); + return ret; } void __lockfunc tty_unlock(struct tty_struct *tty) -- 2.7.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-05 18:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-03 16:10 tty: tty_struct memory leak Dmitry Vyukov 2016-02-03 16:26 ` Dmitry Vyukov 2016-02-03 23:27 ` Peter Hurley 2016-02-04 10:48 ` Dmitry Vyukov 2016-02-04 21:48 ` Peter Hurley 2016-02-05 18:49 ` [PATCH] tty: Drop krefs for interrupted tty lock Peter Hurley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).