Skip to contentSkip to conversation

tty: tty_struct memory leak

PATCH New Dmitry Vyukov 2 participants 6 messages Feb 3, 2016 – Feb 5, 2016 TTY layer and serial drivers

lore
Share & tools
lore

Dmitry Vyukov

author
#0 lore raw

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

author
#1 lore raw
Quoted 39 lines · Dmitry Vyukov

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
Quoted 8 lines

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

#2 lore raw

Hi Dmitry,

Quoted 12 lines · Dmitry Vyukov
Quoted 11 lines · Dmitry Vyukov

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>

Quoted 6 lines
Quoted 6 lines

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

Quoted 23 lines
Quoted 23 lines

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

Quoted 8 lines
Quoted 5 lines
# 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(-)
drivers/tty/tty_io.c +3 -1
index 3f4f47a..15f2d6d 100644
@@ -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
Signature · 1 line
2.7.0
author
#3 lore raw
Quoted 46 lines · Peter Hurley

Hi Dmitry,

Quoted 12 lines · Dmitry Vyukov
Quoted 11 lines · Dmitry Vyukov

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>

Quoted 6 lines
Quoted 6 lines

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
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 1 line

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.

#4 lore raw
Quoted 47 lines · Dmitry Vyukov
Quoted 46 lines · Peter Hurley

Hi Dmitry,

Quoted 12 lines · Dmitry Vyukov
Quoted 11 lines · Dmitry Vyukov

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>

Quoted 6 lines
Quoted 6 lines

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
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 4 lines
Quoted 1 line

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.

Quoted 39 lines

// 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.

#5 lore raw

PATCH 1/1 tty: Drop krefs for interrupted tty lock

In reply to Dmitry Vyukov

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 <[email protected]>
Signed-off-by Peter Hurley <[email protected]>

---

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(-)
drivers/tty/tty_io.c +1 -2
index 5cec01c..a7eacef 100644
@@ -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);
drivers/tty/tty_mutex.c +6 -1
index d2f3c4c..dfa9ec0 100644
@@ -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)
Signature · 1 line
2.7.0