Re: memory leak in pty_common_install

From: Jiri Slaby
Date: Mon Aug 05 2019 - 05:11:08 EST


On 04. 08. 19, 16:23, Hillf Danton wrote:
>
> On Tue, 30 Jul 2019 08:08:05 -0700
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: 6789f873 Merge tag 'pm-5.3-rc2' of git://git.kernel.org/pu..
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1696897c600000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=339b6a6b3640d115
>> dashboard link: https://syzkaller.appspot.com/bug?extid=bdebcbf44250d75bdd82
>> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=153d7544600000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+bdebcbf44250d75bdd82@xxxxxxxxxxxxxxxxxxxxxxxxx
>>
>> BUG: memory leak
>> unreferenced object 0xffff88810d84d400 (size 512):
>> comm "syz-executor.5", pid 7522, jiffies 4294954305 (age 14.260s)
>> hex dump (first 32 bytes):
>> 50 d4 84 0d 81 88 ff ff e0 ff ff ff 0f 00 00 00 P...............
>> 10 d4 84 0d 81 88 ff ff 10 d4 84 0d 81 88 ff ff ................
>> backtrace:
>> [<000000003d61da44>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
>> [<000000003d61da44>] slab_post_alloc_hook mm/slab.h:522 [inline]
>> [<000000003d61da44>] slab_alloc mm/slab.c:3319 [inline]
>> [<000000003d61da44>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
>> [<00000000a6239e0a>] kmalloc include/linux/slab.h:552 [inline]
>> [<00000000a6239e0a>] pty_common_install+0x4e/0x2b0 drivers/tty/pty.c:391
>> [<00000000bd8cb19d>] pty_unix98_install+0x20/0x30 drivers/tty/pty.c:740
>> [<000000001b46b5e1>] tty_driver_install_tty drivers/tty/tty_io.c:1227 [inline]
>> [<000000001b46b5e1>] tty_init_dev drivers/tty/tty_io.c:1340 [inline]
>> [<000000001b46b5e1>] tty_init_dev+0x86/0x210 drivers/tty/tty_io.c:1317
>> [<00000000845ae712>] ptmx_open drivers/tty/pty.c:845 [inline]
>> [<00000000845ae712>] ptmx_open+0xba/0x1c0 drivers/tty/pty.c:811
>> [<000000007e87d771>] chrdev_open+0xe3/0x290 fs/char_dev.c:414
>> [<00000000bd556826>] do_dentry_open+0x199/0x4f0 fs/open.c:797
>> [<000000001ba9145b>] vfs_open+0x35/0x40 fs/open.c:906
>> [<00000000c0275eb4>] do_last fs/namei.c:3416 [inline]
>> [<00000000c0275eb4>] path_openat+0x854/0x1cd0 fs/namei.c:3533
>> [<00000000156ad8b1>] do_filp_open+0xaa/0x130 fs/namei.c:3563
>> [<00000000074d96c0>] do_sys_open+0x253/0x330 fs/open.c:1089
>> [<000000009f7fc64a>] __do_sys_openat fs/open.c:1116 [inline]
>> [<000000009f7fc64a>] __se_sys_openat fs/open.c:1110 [inline]
>> [<000000009f7fc64a>] __x64_sys_openat+0x24/0x30 fs/open.c:1110
>> [<000000005ca4479f>] do_syscall_64+0x76/0x1a0 arch/x86/entry/common.c:296
>> [<00000000e1f64b0f>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>
>> BUG: memory leak
>> unreferenced object 0xffff88810e639800 (size 1024):
>> comm "syz-executor.5", pid 7522, jiffies 4294954305 (age 14.260s)
>> hex dump (first 32 bytes):
>> 01 54 00 00 01 00 00 00 00 00 00 00 00 00 00 00 .T..............
>> 00 83 fa 19 82 88 ff ff a0 7f 9b 83 ff ff ff ff ................
>> backtrace:
>> [<000000003d61da44>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
>> [<000000003d61da44>] slab_post_alloc_hook mm/slab.h:522 [inline]
>> [<000000003d61da44>] slab_alloc mm/slab.c:3319 [inline]
>> [<000000003d61da44>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
>> [<000000001cfffc30>] kmalloc include/linux/slab.h:552 [inline]
>> [<000000001cfffc30>] kzalloc include/linux/slab.h:748 [inline]
>> [<000000001cfffc30>] alloc_tty_struct+0x3f/0x290 drivers/tty/tty_io.c:2981
>> [<000000001946a70c>] pty_common_install+0xac/0x2b0 drivers/tty/pty.c:399
>> [<00000000bd8cb19d>] pty_unix98_install+0x20/0x30 drivers/tty/pty.c:740
>> [<000000001b46b5e1>] tty_driver_install_tty drivers/tty/tty_io.c:1227 [inline]
>> [<000000001b46b5e1>] tty_init_dev drivers/tty/tty_io.c:1340 [inline]
>> [<000000001b46b5e1>] tty_init_dev+0x86/0x210 drivers/tty/tty_io.c:1317
>> [<00000000845ae712>] ptmx_open drivers/tty/pty.c:845 [inline]
>> [<00000000845ae712>] ptmx_open+0xba/0x1c0 drivers/tty/pty.c:811
>> [<000000007e87d771>] chrdev_open+0xe3/0x290 fs/char_dev.c:414
>> [<00000000bd556826>] do_dentry_open+0x199/0x4f0 fs/open.c:797
>> [<000000001ba9145b>] vfs_open+0x35/0x40 fs/open.c:906
>> [<00000000c0275eb4>] do_last fs/namei.c:3416 [inline]
>> [<00000000c0275eb4>] path_openat+0x854/0x1cd0 fs/namei.c:3533
>> [<00000000156ad8b1>] do_filp_open+0xaa/0x130 fs/namei.c:3563
>> [<00000000074d96c0>] do_sys_open+0x253/0x330 fs/open.c:1089
>> [<000000009f7fc64a>] __do_sys_openat fs/open.c:1116 [inline]
>> [<000000009f7fc64a>] __se_sys_openat fs/open.c:1110 [inline]
>> [<000000009f7fc64a>] __x64_sys_openat+0x24/0x30 fs/open.c:1110
>> [<000000005ca4479f>] do_syscall_64+0x76/0x1a0 arch/x86/entry/common.c:296
>> [<00000000e1f64b0f>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Reset tty for port if none cares the open result.

Could you elaborate on how tty_port_open affects ptys? I.e. how does
this report relates to the change below?

And why do you think it is necessary at all? tty_port_close should take
care of it.

> --- a/drivers/tty/tty_port.c
> +++ b/drivers/tty/tty_port.c
> @@ -669,6 +669,8 @@ EXPORT_SYMBOL_GPL(tty_port_install);
> int tty_port_open(struct tty_port *port, struct tty_struct *tty,
> struct file *filp)
> {
> + int retval = 0;
> +
> spin_lock_irq(&port->lock);
> ++port->count;
> spin_unlock_irq(&port->lock);
> @@ -685,16 +687,18 @@ int tty_port_open(struct tty_port *port,
> if (!tty_port_initialized(port)) {
> clear_bit(TTY_IO_ERROR, &tty->flags);
> if (port->ops->activate) {
> - int retval = port->ops->activate(port, tty);
> - if (retval) {
> - mutex_unlock(&port->mutex);
> - return retval;
> - }
> + retval = port->ops->activate(port, tty);
> + if (retval)
> + goto out;
> }
> tty_port_set_initialized(port, 1);
> }
> +out:
> mutex_unlock(&port->mutex);
> - return tty_port_block_til_ready(port, tty, filp);
> + if (!retval)
> + retval = tty_port_block_til_ready(port, tty, filp);
> + if (retval)
> + tty_port_tty_set(port, 0);
> + return retval;
> }
> -
> EXPORT_SYMBOL(tty_port_open);
> --
>
>

thanks,
--
js
suse labs