* 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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.