* 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).