Re: tty: tty_struct memory leak

[Dmitry Vyukov]

On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvy...@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

[Peter Hurley]

Hi Dmitry,

On 02/03/2016 08:26 AM, Dmitry Vyukov wrote:
> On Wed, Feb 3, 2016 at 5:10 PM, Dmitry Vyukov <dvy...@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

[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

[Dmitry Vyukov]

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.

[Peter Hurley]

Yes, I see the bug now, thanks.