BUG in tty_open when using containers and ptrace

BUG in tty_open when using containers and ptrace

[Môshe van der Sterre]

Hello,

I am working on the lxc userspace tools, and got this BUG a couple of
times.
Here are the reports:

http://moshe.nl/tty-bugshot1.png
http://moshe.nl/tty-bugshot2.png

I am running 2.6.29.1 from kernel.org in a virtual machine under kvm.
I have not been able to reliably reproduce it, but it happens only after
the container is setup and ptrace is active on lxc-start.
I think the first screen shot is while all ttys where allowed in
cgroup.devices, the second one while only tty1 was allowed.

Greetings,
Môshe van der Sterre
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

BUG in tty_open when using containers and ptrace

[Môshe van der Sterre]

Hello,

I am working on the lxc userspace tools, and got this BUG a couple of
times.
Here are the reports:

http://moshe.nl/tty-bugshot1.png
http://moshe.nl/tty-bugshot2.png

I am running 2.6.29.1 from kernel.org in a virtual machine under kvm.
I have not been able to reliably reproduce it, but it happens only after
the container is setup and ptrace is active on lxc-start.
I think the first screen shot is while all ttys where allowed in
cgroup.devices, the second one while only tty1 was allowed.

Greetings,
Môshe van der Sterre

Re: BUG in tty_open when using containers and ptrace

[Serge E. Hallyn]

Quoting Môshe van der Sterre (me-A/3C56C7qwM@public.gmane.org):
> Hello,
> 
> I am working on the lxc userspace tools, and got this BUG a couple of
> times.
> Here are the reports:
> 
> http://moshe.nl/tty-bugshot1.png
> http://moshe.nl/tty-bugshot2.png
> 
> I am running 2.6.29.1 from kernel.org in a virtual machine under kvm.
> I have not been able to reliably reproduce it, but it happens only after
> the container is setup and ptrace is active on lxc-start.
> I think the first screen shot is while all ttys where allowed in
> cgroup.devices, the second one while only tty1 was allowed.

Which distro is this - an uptodate F10, fresh F11, something else?

Since it also happens when all ttys are allowed it probably isn't the
devices cgroup...  Actually the devpts code seems the most likely
cause.  Suka, can you take a look at the 2.6.29.1 source for anything
suspect?

Môshe, can you send me tty_io.S after doing:
	'objdump -d drivers/char/tty_io.o > tty_io.S'

as well as strace_output after doing:
	'strace -f -ostrace_output <your_lxc_start_command_here>'
?

thanks,
-serge

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

Hi,

On pon, kwi 13, 2009 at 09:20:38 -0500, Serge E. Hallyn wrote:
> Quoting Môshe van der Sterre (me@moshe.nl):
> > Hello,
> > 
> > I am working on the lxc userspace tools, and got this BUG a couple of
> > times.
> > Here are the reports:
> > 
> > http://moshe.nl/tty-bugshot1.png
> > http://moshe.nl/tty-bugshot2.png
> > 
> > I am running 2.6.29.1 from kernel.org in a virtual machine under kvm.
> > I have not been able to reliably reproduce it, but it happens only after
> > the container is setup and ptrace is active on lxc-start.
> > I think the first screen shot is while all ttys where allowed in
> > cgroup.devices, the second one while only tty1 was allowed.
> 
> Which distro is this - an uptodate F10, fresh F11, something else?
> 
> Since it also happens when all ttys are allowed it probably isn't the
> devices cgroup...  Actually the devpts code seems the most likely
> cause.  Suka, can you take a look at the 2.6.29.1 source for anything
> suspect?
> 
> Môshe, can you send me tty_io.S after doing:
> 	'objdump -d drivers/char/tty_io.o > tty_io.S'
> 
> as well as strace_output after doing:
> 	'strace -f -ostrace_output <your_lxc_start_command_here>'
> ?

Sorry for digging up such an old thread but I have apparently been
bitten by the same bug. I've been playing with libvirt 0.6.4 (which
mounts devpts with option newinstance) and I got the oops below
while strace'ing container init (upstart actually, got SIGINT and was
about to exit; sorry, no strace available but I remember nothing
extraordinary except for a "+++ Killed by SIGSEGV +++" at the end).

The kernel was vanilla 2.6.30 (hand-rolled on Debian Lenny).

The machine is quite uncomfortable to reboot but is not in production
use, so I guess I can test some patches if needed.

Best regards,
 Grzegorz Nosek

Jul  3 23:23:31 sback kernel: [170809.413989] general protection fault: 0000 [#1] SMP
Jul  3 23:23:31 sback kernel: [170809.414024] last sysfs file: /sys/class/net/lo/operstate
Jul  3 23:23:31 sback kernel: [170809.414052] CPU 0
Jul  3 23:23:31 sback kernel: [170809.414074] Modules linked in: veth ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc ipv6 w
83627hf lm85 hwmon_vid loop evdev tg3 libphy k8temp shpchp pci_hotplug i2c_nforce2 i2c_core container thermal processor thermal_sys button
Jul  3 23:23:31 sback kernel: [170809.414272] Pid: 13974, comm: init Not tainted 2.6.30-sback #3 S2891
Jul  3 23:23:31 sback kernel: [170809.414301] RIP: 0010:[<ffffffff80437914>]  [<ffffffff80437914>] tty_open+0x1fb/0x423
Jul  3 23:23:31 sback kernel: [170809.414354] RSP: 0018:ffff880068015cf8  EFLAGS: 00010246
Jul  3 23:23:31 sback kernel: [170809.414383] RAX: ffff88007e540800 RBX: ffff88007e540800 RCX: ffff88007e8cd708
Jul  3 23:23:31 sback kernel: [170809.414428] RDX: 6973646e65732f64 RSI: 0000000000000000 RDI: ffff880058c8dc10
Jul  3 23:23:31 sback kernel: [170809.414474] RBP: ffff880068015d48 R08: 0000000000000002 R09: 0000000000000000
Jul  3 23:23:31 sback kernel: [170809.414519] R10: 0000000000000167 R11: 0000000000000004 R12: ffff88013ede9000
Jul  3 23:23:31 sback kernel: [170809.414564] R13: ffff88013ede9008 R14: ffff88007e586c00 R15: 0000000000000100
Jul  3 23:23:31 sback kernel: [170809.414609] FS:  00007f4a887056e0(0000) GS:ffffc20000000000(0000) knlGS:0000000000000000
Jul  3 23:23:31 sback kernel: [170809.414656] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
Jul  3 23:23:31 sback kernel: [170809.414685] CR2: 00000000020cd000 CR3: 000000007dc1a000 CR4: 00000000000006e0
Jul  3 23:23:31 sback kernel: [170809.414730] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Jul  3 23:23:31 sback kernel: [170809.414775] DR3: 0000000000000000 DR6: 00000000ffff4ff0 DR7: 0000000000000400
Jul  3 23:23:31 sback kernel: [170809.414821] Process init (pid: 13974, threadinfo ffff880068014000, task ffff880072560040)
Jul  3 23:23:31 sback kernel: [170809.414868] Stack:
Jul  3 23:23:31 sback kernel: [170809.414889]  ffff88007e586c00 ffff880058c8dc10 0880000000008101 0000000000000001
Jul  3 23:23:31 sback kernel: [170809.414922]  0000000000000000 0000000000000000 0000000000000000 ffff88013ede9008
Jul  3 23:23:31 sback kernel: [170809.414971]  0000000000000000 ffff880058c8dc10 ffff880068015d98 ffffffff802d5b25
Jul  3 23:23:31 sback kernel: [170809.415036] Call Trace:
Jul  3 23:23:31 sback kernel: [170809.415059]  [<ffffffff802d5b25>] chrdev_open+0x15f/0x17e
Jul  3 23:23:31 sback kernel: [170809.415090]  [<ffffffff803baa3b>] ? selinux_dentry_open+0xf2/0xfb
Jul  3 23:23:31 sback kernel: [170809.415125]  [<ffffffff802d59c6>] ? chrdev_open+0x0/0x17e
Jul  3 23:23:31 sback kernel: [170809.415154]  [<ffffffff802d1082>] __dentry_open+0x155/0x274
Jul  3 23:23:31 sback kernel: [170809.415186]  [<ffffffff802d1278>] nameidata_to_filp+0x46/0x57
Jul  3 23:23:31 sback kernel: [170809.415218]  [<ffffffff802deb1b>] do_filp_open+0x4ca/0x924
Jul  3 23:23:31 sback kernel: [170809.415251]  [<ffffffff802e7ce8>] ? alloc_fd+0x122/0x133
Jul  3 23:23:31 sback kernel: [170809.415281]  [<ffffffff802d0e61>] do_sys_open+0x5b/0xdb
Jul  3 23:23:31 sback kernel: [170809.415312]  [<ffffffff802d0f14>] sys_open+0x20/0x22
Jul  3 23:23:31 sback kernel: [170809.415341]  [<ffffffff8020bf36>] tracesys+0xd9/0xde
Jul  3 23:23:31 sback kernel: [170809.415373] Code: 81 fb 00 f0 ff ff 76 11 48 c7 c7 60 61 7d 80 e8 c1 38 17 00 e9 a9 00 00 00 48 85 db 74 5c 80 bb 40 01 00 00 00 48 8b 53 08 78 64 <81> ba 9c 00 00 00 04 00 01 00 75 16 83 bb 48 01 00 0
0 00 75 4f
Jul  3 23:23:31 sback kernel: [170809.415582] RIP  [<ffffffff80437914>] tty_open+0x1fb/0x423
Jul  3 23:23:31 sback kernel: [170809.415612]  RSP <ffff880068015cf8>
Jul  3 23:23:31 sback kernel: [170809.415869] ---[ end trace e65e6319b2f34f5a ]---


Decoding the code yields:
All code
========
   0:   81 fb 00 f0 ff ff       cmp    $0xfffff000,%ebx
   6:   76 11                   jbe    0x19
   8:   48 c7 c7 60 61 7d 80    mov    $0xffffffff807d6160,%rdi
   f:   e8 c1 38 17 00          callq  0x1738d5
  14:   e9 a9 00 00 00          jmpq   0xc2
  19:   48 85 db                test   %rbx,%rbx
  1c:   74 5c                   je     0x7a
  1e:   80 bb 40 01 00 00 00    cmpb   $0x0,0x140(%rbx)
  25:   48 8b 53 08             mov    0x8(%rbx),%rdx
  29:   78 64                   js     0x8f
  2b:*  81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)     <-- trapping instruction
  32:   00 01 00
  35:   75 16                   jne    0x4d
  37:   83                      .byte 0x83
  38:   bb 48 01 00 00          mov    $0x148,%ebx

Code starting with the faulting instruction
===========================================
   0:   81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)
   7:   00 01 00
   a:   75 16                   jne    0x22
   c:   83                      .byte 0x83
   d:   bb 48 01 00 00          mov    $0x148,%ebx


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Sat, Jul 04, 2009 at 03:28:52PM +0200, Grzegorz Nosek wrote:
> Decoding the code yields:
> All code
> ========
>    0:   81 fb 00 f0 ff ff       cmp    $0xfffff000,%ebx
>    6:   76 11                   jbe    0x19
>    8:   48 c7 c7 60 61 7d 80    mov    $0xffffffff807d6160,%rdi
>    f:   e8 c1 38 17 00          callq  0x1738d5
>   14:   e9 a9 00 00 00          jmpq   0xc2
>   19:   48 85 db                test   %rbx,%rbx
>   1c:   74 5c                   je     0x7a
>   1e:   80 bb 40 01 00 00 00    cmpb   $0x0,0x140(%rbx)
>   25:   48 8b 53 08             mov    0x8(%rbx),%rdx
>   29:   78 64                   js     0x8f
>   2b:*  81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)     <-- trapping instruction
>   32:   00 01 00
>   35:   75 16                   jne    0x4d
>   37:   83                      .byte 0x83
>   38:   bb 48 01 00 00          mov    $0x148,%ebx
> 
> Code starting with the faulting instruction
> ===========================================
>    0:   81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)
>    7:   00 01 00
>    a:   75 16                   jne    0x22
>    c:   83                      .byte 0x83
>    d:   bb 48 01 00 00          mov    $0x148,%ebx

To my untrained eye it looks like the cmpl corresponds to:

1841        if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
1842            tty->driver->subtype == PTY_TYPE_MASTER)

in drivers/char/tty_io.c

That means that %rdx should contain tty->driver, but contains
0x6973646e65732f64, which looks like a part of '/etc/init.d/sendsigs'.
So, we're possibly using an already freed and overwritten tty struct.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

> That means that %rdx should contain tty->driver, but contains
> 0x6973646e65732f64, which looks like a part of '/etc/init.d/sendsigs'.
> So, we're possibly using an already freed and overwritten tty struct.

Okay, got another one:

Jul  5 13:47:29 sback kernel: [83780.950357] ------------[ cut here ]------------
Jul  5 13:47:29 sback kernel: [83780.950395] WARNING: at drivers/char/tty_io.c:1335 tty_open+0x245/0x423()
Jul  5 13:47:29 sback kernel: [83780.950426] Hardware name: S2891
Jul  5 13:47:29 sback kernel: [83780.950449] Modules linked in: veth ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack ipt_REJECT bridge stp llc sha1_generic xt_SYSRQ compat_xtables ip6_tables xt_tcpudp iptable_filter ip_tables x_tables ipv6 w83627hf lm85 hwmon_vid loop evdev tg3 libphy k8temp shpchp pci_hotplug i2c_nforce2 i2c_core container button thermal processor thermal_sys
Jul  5 13:47:29 sback kernel: [83780.950668] Pid: 32628, comm: init Not tainted 2.6.30-sback #3
Jul  5 13:47:29 sback kernel: [83780.950697] Call Trace:
Jul  5 13:47:29 sback kernel: [83780.950723]  [<ffffffff8043795e>] ? tty_open+0x245/0x423
Jul  5 13:47:29 sback kernel: [83780.950754]  [<ffffffff802399d0>] warn_slowpath_common+0x7c/0xa9
Jul  5 13:47:29 sback kernel: [83780.950785]  [<ffffffff80239a11>] warn_slowpath_null+0x14/0x16
Jul  5 13:47:29 sback kernel: [83780.950815]  [<ffffffff8043795e>] tty_open+0x245/0x423
Jul  5 13:47:29 sback kernel: [83780.950846]  [<ffffffff802d5b25>] chrdev_open+0x15f/0x17e
Jul  5 13:47:29 sback kernel: [83780.950878]  [<ffffffff803baa3b>] ? selinux_dentry_open+0xf2/0xfb
Jul  5 13:47:29 sback kernel: [83780.950908]  [<ffffffff802d59c6>] ? chrdev_open+0x0/0x17e
Jul  5 13:47:29 sback kernel: [83780.950939]  [<ffffffff802d1082>] __dentry_open+0x155/0x274
Jul  5 13:47:29 sback kernel: [83780.950970]  [<ffffffff802d1278>] nameidata_to_filp+0x46/0x57
Jul  5 13:47:29 sback kernel: [83780.951001]  [<ffffffff802deb1b>] do_filp_open+0x4ca/0x924
Jul  5 13:47:29 sback kernel: [83780.951033]  [<ffffffff802e7ce8>] ? alloc_fd+0x122/0x133
Jul  5 13:47:29 sback kernel: [83780.951063]  [<ffffffff802d0e61>] do_sys_open+0x5b/0xdb
Jul  5 13:47:29 sback kernel: [83780.951093]  [<ffffffff802d0f14>] sys_open+0x20/0x22
Jul  5 13:47:29 sback kernel: [83780.951124]  [<ffffffff8020bc9b>] system_call_fastpath+0x16/0x1b
Jul  5 13:47:29 sback kernel: [83780.951154] ---[ end trace b453453d8c153fcc ]---
Jul  5 13:47:29 sback kernel: [83780.951187] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
Jul  5 13:47:29 sback kernel: [83780.951233] IP: [<ffffffff802d387f>] file_move+0x3c/0x55
Jul  5 13:47:29 sback kernel: [83780.951257] PGD 7bc58067 PUD 7bd8c067 PMD 0
Jul  5 13:47:29 sback kernel: [83780.951257] Oops: 0002 [#1] SMP

(ends here, no netconsole or anything and 'ssh tail -f' managed to only
get this far).

I didn't strace anything this time, was playing with libvirt's apparent
mishandling of container shutdown and I (eventually) sent SIGTERM and
then SIGINT to container init, which caused it first to reexec, and then
to exit. Immediately after sending SIGINT the box froze solid.

I remember that I have issued that same sequence when I got the first
crash, so it looks vaguely related.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Môshe van der Sterre]

On Sat, 4 Jul 2009 15:28:52 +0200, Grzegorz Nosek <root@localdomain.pl>
wrote:
> Sorry for digging up such an old thread but I have apparently been
> bitten by the same bug.

Ok.
Unfortunately, I have had no opportunity to put time into this bug,
and have not been able to provide the info Serge requested.

> I've been playing with libvirt 0.6.4 (which
> mounts devpts with option newinstance) and I got the oops below
> while strace'ing container init (upstart actually, got SIGINT and was
> about to exit; sorry, no strace available but I remember nothing
> extraordinary except for a "+++ Killed by SIGSEGV +++" at the end).

Your bug seems the same. I had not noticed it using unmodified lxc tools,
but apparently it is present in your case. 

I'll be following this, and reply if circumstances permit.

Greetings,
Môshe van der Sterre
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On nie, lip 05, 2009 at 10:00:58 +0200, Môshe van der Sterre wrote:
> On Sat, 4 Jul 2009 15:28:52 +0200, Grzegorz Nosek <root@localdomain.pl>
> wrote:
> > Sorry for digging up such an old thread but I have apparently been
> > bitten by the same bug.
> 
> Ok.
> Unfortunately, I have had no opportunity to put time into this bug,
> and have not been able to provide the info Serge requested.
> 
> > I've been playing with libvirt 0.6.4 (which
> > mounts devpts with option newinstance) and I got the oops below
> > while strace'ing container init (upstart actually, got SIGINT and was
> > about to exit; sorry, no strace available but I remember nothing
> > extraordinary except for a "+++ Killed by SIGSEGV +++" at the end).
> 
> Your bug seems the same. I had not noticed it using unmodified lxc tools,
> but apparently it is present in your case. 
> 
> I'll be following this, and reply if circumstances permit.

Can/could you reliably trigger the bug? I managed to kill the box
twice by poking at random but I'm not sure what exact steps are needed.

Best regards,
 Grzegorz Nosek
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| On Sat, Jul 04, 2009 at 03:28:52PM +0200, Grzegorz Nosek wrote:
| > Decoding the code yields:
| > All code
| > ========
| >    0:   81 fb 00 f0 ff ff       cmp    $0xfffff000,%ebx
| >    6:   76 11                   jbe    0x19
| >    8:   48 c7 c7 60 61 7d 80    mov    $0xffffffff807d6160,%rdi
| >    f:   e8 c1 38 17 00          callq  0x1738d5
| >   14:   e9 a9 00 00 00          jmpq   0xc2
| >   19:   48 85 db                test   %rbx,%rbx
| >   1c:   74 5c                   je     0x7a
| >   1e:   80 bb 40 01 00 00 00    cmpb   $0x0,0x140(%rbx)
| >   25:   48 8b 53 08             mov    0x8(%rbx),%rdx
| >   29:   78 64                   js     0x8f
| >   2b:*  81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)     <-- trapping instruction
| >   32:   00 01 00
| >   35:   75 16                   jne    0x4d
| >   37:   83                      .byte 0x83
| >   38:   bb 48 01 00 00          mov    $0x148,%ebx
| > 
| > Code starting with the faulting instruction
| > ===========================================
| >    0:   81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)
| >    7:   00 01 00
| >    a:   75 16                   jne    0x22
| >    c:   83                      .byte 0x83
| >    d:   bb 48 01 00 00          mov    $0x148,%ebx
| 
| To my untrained eye it looks like the cmpl corresponds to:
| 
| 1841        if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
| 1842            tty->driver->subtype == PTY_TYPE_MASTER)
| 
| in drivers/char/tty_io.c

I don't yet have the exact version of tty_io.c so the line numbers don't
match, but even so isn't the above 'cmpl' comparing a constant 0x10004
with the %rdx+0x9c address ? If so, I am not sure how it matches up to
either TTY_DRIVER_TYPE_PTY (0x0004) or PTY_TYPE_MASTER (0x0001).

Wouldn't the above 'if' require two separate compare instructions ?

| 
| That means that %rdx should contain tty->driver, but contains
| 0x6973646e65732f64, which looks like a part of '/etc/init.d/sendsigs'.
| So, we're possibly using an already freed and overwritten tty struct.

Is it possibly related to this bug where they talk of some sort of
corruption going on with tty data structures (kernel versions appear
to be close, but need to double check).

	http://lkml.org/lkml/2009/6/16/131

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On pon, lip 06, 2009 at 08:31:35 -0700, Sukadev Bhattiprolu wrote:
> | To my untrained eye it looks like the cmpl corresponds to:
> | 
> | 1841        if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
> | 1842            tty->driver->subtype == PTY_TYPE_MASTER)
> | 
> | in drivers/char/tty_io.c
> 
> I don't yet have the exact version of tty_io.c so the line numbers don't
> match, but even so isn't the above 'cmpl' comparing a constant 0x10004
> with the %rdx+0x9c address ? If so, I am not sure how it matches up to
> either TTY_DRIVER_TYPE_PTY (0x0004) or PTY_TYPE_MASTER (0x0001).
> 
> Wouldn't the above 'if' require two separate compare instructions ?

They're two 16-bit words occupying the same 32-bit word, so presumably
gcc optimised it into a single memory access.

> Is it possibly related to this bug where they talk of some sort of
> corruption going on with tty data structures (kernel versions appear
> to be close, but need to double check).
> 
> 	http://lkml.org/lkml/2009/6/16/131

2.6.30 has no tty_ldisc_hangup (the function being touched) at all. I
guess it was added while restructuring the tty layer. I'll have a look
at it though, thanks for the link.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On sob, lip 04, 2009 at 04:34:12 +0200, Grzegorz Nosek wrote:
> On Sat, Jul 04, 2009 at 03:28:52PM +0200, Grzegorz Nosek wrote:
> > Decoding the code yields:
> > All code
> > ========
> >    0:   81 fb 00 f0 ff ff       cmp    $0xfffff000,%ebx
> >    6:   76 11                   jbe    0x19
> >    8:   48 c7 c7 60 61 7d 80    mov    $0xffffffff807d6160,%rdi
> >    f:   e8 c1 38 17 00          callq  0x1738d5
> >   14:   e9 a9 00 00 00          jmpq   0xc2
> >   19:   48 85 db                test   %rbx,%rbx
> >   1c:   74 5c                   je     0x7a
> >   1e:   80 bb 40 01 00 00 00    cmpb   $0x0,0x140(%rbx)
> >   25:   48 8b 53 08             mov    0x8(%rbx),%rdx
> >   29:   78 64                   js     0x8f
> >   2b:*  81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)     <-- trapping instruction
> >   32:   00 01 00
> >   35:   75 16                   jne    0x4d
> >   37:   83                      .byte 0x83
> >   38:   bb 48 01 00 00          mov    $0x148,%ebx
> > 
> > Code starting with the faulting instruction
> > ===========================================
> >    0:   81 ba 9c 00 00 00 04    cmpl   $0x10004,0x9c(%rdx)
> >    7:   00 01 00
> >    a:   75 16                   jne    0x22
> >    c:   83                      .byte 0x83
> >    d:   bb 48 01 00 00          mov    $0x148,%ebx
> 
> To my untrained eye it looks like the cmpl corresponds to:
> 
> 1841        if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
> 1842            tty->driver->subtype == PTY_TYPE_MASTER)
> 
> in drivers/char/tty_io.c
> 
> That means that %rdx should contain tty->driver, but contains
> 0x6973646e65732f64, which looks like a part of '/etc/init.d/sendsigs'.
> So, we're possibly using an already freed and overwritten tty struct.

(CC'ing Alan-the-TTY-guru)

OK, so I'm (hopefully) getting somewhere. I can now reliably reproduce
the oops below (with SLUB debugging) by starting and shutting down a
container twice. Tested 2.6.30.1 and 2.6.31-rc2, both invariably crash
at the second shutdown attempt. Afterwards the system is left unstable,
with just about anything touching TTY code hanging.

The container is Debian Lenny with a tiny bit patched upstart from Sid
(more suited to running as container init, i.e. not crashing with an
empty environment and exiting completely upon request). The host is
again Debian Lenny running libvirt 0.6.4 and again, patched a tiny
little bit (fixed container shutdown to actually send a signal to
container init). /dev/pts inside the container is mounted with -o
newinstance, but even with that option patched out from libvirt (or
disabled in the kernel) the oops is identical.

There are several items inside upstart configuration required:
 - mount --bind /dev/pts/0 /dev/console at the very beginning
   (/etc/event.d/rcS running just this one command)
 - /etc/event.d/logd (set up as "stop on runlevel 0" and "console
   output")
 - /etc/event.d/control-alt-delete switching to runlevel 0

I don't really know what does that logd do, but apparently all three
factors are required (e.g. not shutting down logd cleanly but simply
letting it go down with init does not trigger the crash).

Everything is running on a 386 virtualbox VM, but crashes happened also
on a physical amd64 box (see code above).

If needed, I can supply the virtualbox image somewhere (it's 8G, but
I'll strip it down). I can also test just about anything on this VM.

Best regards,
 Grzegorz Nosek

Jul  8 13:53:52 debian kernel: [   31.429837] BUG: unable to handle kernel paging request at 6b6b6bcf
Jul  8 13:53:52 debian kernel: [   31.429837] IP: [<c122c46c>] tty_open+0x11c/0x4b0
Jul  8 13:53:52 debian kernel: [   31.429837] *pde = 00000000
Jul  8 13:53:52 debian kernel: [   31.429837] Oops: 0000 [#1] SMP
Jul  8 13:53:52 debian kernel: [   31.429837] last sysfs file: /sys/class/net/lo/operstate
Jul  8 13:53:52 debian kernel: [   31.429837] Modules linked in: bridge stp llc
Jul  8 13:53:52 debian kernel: [   31.429837]
Jul  8 13:53:52 debian kernel: [   31.429837] Pid: 1595, comm: init Not tainted (2.6.31-rc2 #2) VirtualBox
Jul  8 13:53:52 debian kernel: [   31.429837] EIP: 0060:[<c122c46c>] EFLAGS: 00210202 CPU: 0
Jul  8 13:53:52 debian kernel: [   31.429837] EIP is at tty_open+0x11c/0x4b0
Jul  8 13:53:52 debian kernel: [   31.429837] EAX: ce49b950 EBX: cfaaf780 ECX: ce49b950 EDX: 6b6b6b6b
Jul  8 13:53:52 debian kernel: [   31.429837] ESI: ce49b950 EDI: 08800000 EBP: ce589e64 ESP: ce589e40
Jul  8 13:53:52 debian kernel: [   31.429837]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Jul  8 13:53:52 debian kernel: [   31.429837] Process init (pid: 1595, ti=ce588000 task=ce545380 task.ti=ce588000)
Jul  8 13:53:52 debian kernel: [   31.429837] Stack:
Jul  8 13:53:52 debian kernel: [   31.429837]  cfaaf784 cf309100 cf6930d8 00000102 00000100 00000000 00000000 cfaaf788
Jul  8 13:53:52 debian kernel: [   31.429837] <0> cf6930d8 ce589e84 c1003c92 cf309100 cf309100 00000000 cf309100 00000000
Jul  8 13:53:52 debian kernel: [   31.429837] <0> cf6930d8 ce589ea0 c10e14a3 cf056cc0 cf6a3d48 cf309100 ce589ef0 cf309100
Jul  8 13:53:52 debian kernel: [   31.429837] Call Trace:
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c1003c92>] ? return_to_handler+0x0/0x1e
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10058de>] dump_trace+0x8e/0xe0
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10e14a3>] ? __dentry_open+0xc3/0x280
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10e175b>] ? nameidata_to_filp+0x5b/0x70
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10e6430>] ? chrdev_open+0x0/0x190
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10ef851>] ? do_filp_open+0x271/0x900
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c13f03a4>] ? _spin_unlock+0x4/0x30
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10f9800>] ? alloc_fd+0xe0/0x100
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c101b5e4>] ? prepare_ftrace_return+0x64/0xa0
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c13f03c2>] ? _spin_unlock+0x22/0x30
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10f9800>] ? alloc_fd+0xe0/0x100
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10e126d>] ? do_sys_open+0x6d/0x130
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c10e139e>] ? sys_open+0x2e/0x40
Jul  8 13:53:52 debian kernel: [   31.429837]  [<c1003155>] ? syscall_call+0x7/0xb
Jul  8 13:53:52 debian kernel: [   31.429837] Code: f8 29 d0 8d 53 04 89 45 f0 89 d0 89 55 dc e8 3c 81 f9 ff 85 f6 0f 84 14 02 00 00 80 be 08 01 00 00 00 8b 56 08 0f 88 5c 01 00 00 <81> 7a 64 04 00 01 00 75 1b 8b 86 0c 01 00 00 85 c0 0f 85 45 01
Jul  8 13:53:52 debian kernel: [   31.429837] EIP: [<c122c46c>] tty_open+0x11c/0x4b0 SS:ESP 0068:ce589e40
Jul  8 13:53:52 debian kernel: [   31.429837] CR2: 000000006b6b6bcf
Jul  8 13:53:52 debian kernel: [   31.511274] ---[ end trace 774489600e77c80b ]---

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Wed, Jul 08, 2009 at 12:54:17PM +0200, Grzegorz Nosek wrote:
> Jul  8 13:53:52 debian kernel: [   31.429837] BUG: unable to handle kernel paging request at 6b6b6bcf
> Jul  8 13:53:52 debian kernel: [   31.429837] IP: [<c122c46c>] tty_open+0x11c/0x4b0

With the following (whitespace-damaged etc.) patch applied I can no longer
oops the kernel but there are several issues:

1. A warning occurs (after several dozen start/shutdown cycles):
Warning: dev (pts0) tty->count(2) != #fd's(1) in tty_release_dev
So refcounting is still broken and this patch possibly just papers over
the real bug.

2. There's a memory leak somewhere (don't know if it was there before as
the system hadn't survived long enough to test that) guesstimated at
several KB per container cycle; building with kmemleak to see what
happens.

3. After adding tons of debug statements I saw that the TTY objects
weren't always freed immediately after container shutdown but were
somehow batched (e.g. a single container shut down would cause two or
three previous containers' tty objects to be freed). Increasing the
delay between subsequent cycles from 3 to 10 seconds didn't seem to
affect the batching. On an otherwise unpatched kernel, the crashes
happened right after the 'batched' cleanups.

All feedback really appreciated.

Best regards,
 Grzegorz Nosek

diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index daebe1b..0ca0c1c 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -556,12 +556,23 @@ static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver,
        return tty;
 }

-static void pty_unix98_shutdown(struct tty_struct *tty)
+static void ptm_unix98_shutdown(struct tty_struct *tty)
 {
        /* We have our own method as we don't use the tty index */
        kfree(tty->termios);
 }

+static void pts_unix98_shutdown(struct tty_struct *tty)
+{
+       struct inode *ino = (struct inode *)tty->driver_data;
+
+       /* We have our own method as we don't use the tty index */
+       kfree(tty->termios);
+
+       if (ino)
+               ino->i_private = NULL;
+}
+
 /* We have no need to install and remove our tty objects as devpts does all
    the work for us */

@@ -633,7 +644,7 @@ static const struct tty_operations ptm_unix98_ops = {
        .unthrottle = pty_unthrottle,
        .set_termios = pty_set_termios,
        .ioctl = pty_unix98_ioctl,
-       .shutdown = pty_unix98_shutdown,
+       .shutdown = ptm_unix98_shutdown,
        .resize = pty_resize
 };

@@ -649,7 +660,7 @@ static const struct tty_operations pty_unix98_ops = {
        .chars_in_buffer = pty_chars_in_buffer,
        .unthrottle = pty_unthrottle,
        .set_termios = pty_set_termios,
-       .shutdown = pty_unix98_shutdown
+       .shutdown = pts_unix98_shutdown
 };

 /**

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Sat, Jul 11, 2009 at 09:30:55PM +0200, Grzegorz Nosek wrote:
> With the following (whitespace-damaged etc.) patch applied I can no longer
> oops the kernel but there are several issues:
> 
> 1. A warning occurs (after several dozen start/shutdown cycles):
> Warning: dev (pts0) tty->count(2) != #fd's(1) in tty_release_dev
> So refcounting is still broken and this patch possibly just papers over
> the real bug.
> 
> 2. There's a memory leak somewhere (don't know if it was there before as
> the system hadn't survived long enough to test that) guesstimated at
> several KB per container cycle; building with kmemleak to see what
> happens.
> 
> 3. After adding tons of debug statements I saw that the TTY objects
> weren't always freed immediately after container shutdown but were
> somehow batched (e.g. a single container shut down would cause two or
> three previous containers' tty objects to be freed). Increasing the
> delay between subsequent cycles from 3 to 10 seconds didn't seem to
> affect the batching. On an otherwise unpatched kernel, the crashes
> happened right after the 'batched' cleanups.

Talking to myself (again), kmemleak doesn't report anything (apart from
a few unrelated early-init warnings) but running the startup/shutdown loop
on tty1 causes totally crazy behaviour on every VT I used. By "totally
crazy" I mean not responding properly to arrow keys, backspace etc., but
printing stuff like ^[B for arrows and a dot for backspace. Note: this
happened _most_ of the time but not _all_ (navigating shell history was
fun). Even the shell started to misbehave as I tried to execute:
echo 3 > drop_caches
and the shell helpfully said:
3 > drop_caches

Any ideas, anyone?

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Alan Cox]

> Talking to myself (again), kmemleak doesn't report anything (apart from
> a few unrelated early-init warnings) but running the startup/shutdown loop
> on tty1 causes totally crazy behaviour on every VT I used. By "totally
> crazy" I mean not responding properly to arrow keys, backspace etc., but

You are only seeing this with containers ?

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On sob, lip 11, 2009 at 11:19:35 +0100, Alan Cox wrote:
> > Talking to myself (again), kmemleak doesn't report anything (apart from
> > a few unrelated early-init warnings) but running the startup/shutdown loop
> > on tty1 causes totally crazy behaviour on every VT I used. By "totally
> > crazy" I mean not responding properly to arrow keys, backspace etc., but
> 
> You are only seeing this with containers ?

Well, that's how I could reproduce it, didn't try any other way.
Multiple devpts instances don't matter here, but quite possibly
namespaces do.

If I let the loop run unattended, after it finishes the VTs are fine,
even the tty1 where the loop ran. However, if I try to use them with the
loop on another VT, they break and stay broken afterwards. Looks like
the container init (which has /dev/pts/0 mounted over /dev/console)
somehow interacts with the current VT. This may be a container isolation
problem rather than a TTY layer bug, though.

Didn't try (yet) running the loop while logged in via ssh or in a mc
subshell (so via a pty). I wonder if that changes anything.

BTW, kmemleak *did* report something after all but it was
networking-related (got the traces).

BTW2, I could not reproduce the fd count error while leaving the VM
unattended (and not letting its tty ops interleave with my tty ops),
so could you please have a look and say if the patch stands on its own?

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Serge E. Hallyn]

Quoting Grzegorz Nosek (root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org):
> > You are only seeing this with containers ?
> 
> Well, that's how I could reproduce it, didn't try any other way.
> Multiple devpts instances don't matter here, but quite possibly
> namespaces do.

I've tried but failed to reproduce.  Do you have a script or a more
precise recipe you can share?

thanks,
-serge

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Mon, Jul 13, 2009 at 02:02:11PM -0500, Serge E. Hallyn wrote:
> I've tried but failed to reproduce.  Do you have a script or a more
> precise recipe you can share?

I do have a script but all it does is starting and shutting down
a container in a loop. I can strip down and publish the virtualbox vm
image somewhere, if that helps.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Serge E. Hallyn]

Quoting Grzegorz Nosek (root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org):
> On Mon, Jul 13, 2009 at 02:02:11PM -0500, Serge E. Hallyn wrote:
> > I've tried but failed to reproduce.  Do you have a script or a more
> > precise recipe you can share?
> 
> I do have a script but all it does is starting and shutting down
> a container in a loop. I can strip down and publish the virtualbox vm
> image somewhere, if that helps.

The container init is /bin/sh, and you
while [ 1 ]; do
	virsh --connect lxc:/// start vm1
	virsh --connect lxc:/// destroy vm1
done

?  (I thought there was something about stracing the container
init too, so i was basically doing the above while also
stracing the bash shell)

-serge

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Mon, Jul 13, 2009 at 03:26:10PM -0500, Serge E. Hallyn wrote:
> The container init is /bin/sh, and you
> while [ 1 ]; do
> 	virsh --connect lxc:/// start vm1
> 	virsh --connect lxc:/// destroy vm1
> done
> 
> ?  (I thought there was something about stracing the container
> init too, so i was basically doing the above while also
> stracing the bash shell)

That was the first occurrence of the crash (though I can't tell you
whether I used sash or upstart then). When I could reproduce the crash
reliably, I was using upstart without strace and the script was
basically

while [ 1 ]; do
	virsh --connect lxc:/// start vm1
	sleep 3
	virsh --connect lxc:/// shutdown vm1
	sleep 3
done

to let the startup/shutdown settle (also, SLUB debugging had to be on to
poison freed memory -- this was crucial for me).

OK, I've been writing this mail for over an hour now, but I managed to
produce a single program which produces the oops I've been encountering.

Simply run it as container init. Sometimes it oopses immediately,
usually it goes as far as the pause. If you kill it with SIGINT then,
everything will probably be fine. But if you first kill the libvirt_lxc
process above it (holding the pty master), and only then kill init, it
should oops.

Note that the weird mount stuff is critical here (couldn't reproduce the
oops by simply opening /dev/pts/0, even if I mounted it on /dev/console
before).

After applying my patch from some earlier mail I can no longer crash the
kernel, but the "Hello world!" appears on my current VT, so there's
potential for container root to mess up host console. I don't know how
that happened, actually.

Best regards,
 Grzegorz Nosek

/*------------------------------------------------------------------*/
#include <sys/mount.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/signal.h>

void dummy(int sig)
{
}

int main(void)
{
	sleep(2); /* wait for /dev/pts/0 to appear */
	close(0);
	close(1);
	close(2);
	mount("/dev/pts/0", "/dev/console", NULL, MS_BIND, NULL);
	signal(SIGINT, dummy);
	pause();

	open("/dev/console", O_RDWR);
	dup(0);
	dup(0);
	write(1, "Hello world!\n", sizeof("Hello world!\n")-1);
	return 0;
}

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| Simply run it as container init. Sometimes it oopses immediately,

I am trying to reproduce this too and just trying to make sure I get
your environment correctly.  I have just built/installed libvirt 0.6.5
on Ubuntu 9.04. 

The bug does require libvirt+kvm right ?  IOW, does this simple test repro
if it is run as a container-init with say lxc-start (i.e from liblxc - see
http://lxc.sourceforge.net/) ? Another simpler way to create container-init
is the ns_exec program - I assume that won't create the problem ?

Also in an earlier mail you mentioned that disabling the 'newinstance'
mount option to devpts did not help. So does your setup work with older
kernels like 2.6.28 ?

| usually it goes as far as the pause. If you kill it with SIGINT then,
| everything will probably be fine. But if you first kill the libvirt_lxc
| process above it (holding the pty master), and only then kill init, it
| should oops.
| 
| Note that the weird mount stuff is critical here (couldn't reproduce the
| oops by simply opening /dev/pts/0, even if I mounted it on /dev/console
| before).
| 
| After applying my patch from some earlier mail I can no longer crash the
| kernel, but the "Hello world!" appears on my current VT, so there's
| potential for container root to mess up host console. I don't know how
| that happened, actually.
| 
| Best regards,
|  Grzegorz Nosek
| 
| /*------------------------------------------------------------------*/
| #include <sys/mount.h>
| #include <fcntl.h>
| #include <unistd.h>
| #include <sys/signal.h>
| 
| void dummy(int sig)
| {
| }
| 
| int main(void)
| {
| 	sleep(2); /* wait for /dev/pts/0 to appear */
| 	close(0);
| 	close(1);
| 	close(2);
| 	mount("/dev/pts/0", "/dev/console", NULL, MS_BIND, NULL);
| 	signal(SIGINT, dummy);
| 	pause();
| 
| 	open("/dev/console", O_RDWR);
| 	dup(0);
| 	dup(0);
| 	write(1, "Hello world!\n", sizeof("Hello world!\n")-1);
| 	return 0;
| }

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On pon, lip 13, 2009 at 11:49:05 -0700, Sukadev Bhattiprolu wrote:
> Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
> | Simply run it as container init. Sometimes it oopses immediately,
> 
> I am trying to reproduce this too and just trying to make sure I get
> your environment correctly.  I have just built/installed libvirt 0.6.5
> on Ubuntu 9.04. 

0.6.4 or 0.6.5, no difference really (tested both). Host distro is
Debian Lenny but Ubuntu is probably close enough. Didn't test distro
kernel, only used hand-built 2.6.30, 2.6.30.1 and 2.6.31-rc2

> The bug does require libvirt+kvm right ?  IOW, does this simple test repro
> if it is run as a container-init with say lxc-start (i.e from liblxc - see
> http://lxc.sourceforge.net/) ? Another simpler way to create container-init
> is the ns_exec program - I assume that won't create the problem ?

Probably, as it requires a "cross-namespace" pty. I didn't yet set up a
lxc config file. Maybe you can help off the top of your head with the
right incantations? BTW, where's the canonical source for ns_exec?

> Also in an earlier mail you mentioned that disabling the 'newinstance'
> mount option to devpts did not help. So does your setup work with older
> kernels like 2.6.28 ?

Well, I failed to build 2.6.28 with NET_NS (even without SYSFS it wasn't
available in menuconfig) but 2.6.29 exhibits very similar behaviour,
i.e. crashes instantly. The difference is that when the pty master
exits, container init receives a SIGHUP. If it chooses to ignore it and
open /dev/console anyway, *boom*. Looking roughly at %eip and the
faulting address, it's the same bug.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| On pon, lip 13, 2009 at 11:49:05 -0700, Sukadev Bhattiprolu wrote:
| > Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| > | Simply run it as container init. Sometimes it oopses immediately,
| > 
| > I am trying to reproduce this too and just trying to make sure I get
| > your environment correctly.  I have just built/installed libvirt 0.6.5
| > on Ubuntu 9.04. 
| 
| 0.6.4 or 0.6.5, no difference really (tested both). Host distro is
| Debian Lenny but Ubuntu is probably close enough. Didn't test distro
| kernel, only used hand-built 2.6.30, 2.6.30.1 and 2.6.31-rc2

Ok.  Will try 2.6.31-rc2.

| 
| > The bug does require libvirt+kvm right ?  IOW, does this simple test repro
| > if it is run as a container-init with say lxc-start (i.e from liblxc - see
| > http://lxc.sourceforge.net/) ? Another simpler way to create container-init
| > is the ns_exec program - I assume that won't create the problem ?
| 
| Probably, as it requires a "cross-namespace" pty.  I didn't yet set up a
| lxc config file. Maybe you can help off the top of your head with the
| right incantations?

I don't have any beyond what is in the lxc-source examples. Maybe
Daniel Lezcano has some.

|BTW, where's the canonical source for ns_exec?

It is here: git://git.sr71.net/~hallyn/cr_tests.git
| 
| > Also in an earlier mail you mentioned that disabling the 'newinstance'
| > mount option to devpts did not help. So does your setup work with older
| > kernels like 2.6.28 ?
| 
| Well, I failed to build 2.6.28 with NET_NS (even without SYSFS it wasn't
| available in menuconfig) but 2.6.29 exhibits very similar behaviour,
| i.e. crashes instantly. The difference is that when the pty master
| exits, container init receives a SIGHUP. If it chooses to ignore it and
| open /dev/console anyway, *boom*. Looking roughly at %eip and the
| faulting address, it's the same bug.

So I guess this is the first time we are trying this scenario out (i.e.
it may not be a regression so we can't necessarily use git-bisect).

I am not too familiar with libvirt implementation, but do you think it
is possible to repro this using ns_exec and fewer/minimal namespaces ?

We should be able to get a cross-namespace pty using ns_exec, but am not
sure the set of namespaces need to be cloned -  would it be sufficient if
we clone pid and mount namespaces and remount /dev/pts in child container ?

Thanks,

Sukadev

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

Hi,

On wto, lip 14, 2009 at 09:47:44 -0700, Sukadev Bhattiprolu wrote:
> I don't have any beyond what is in the lxc-source examples. Maybe
> Daniel Lezcano has some.

Just asking :)

> |BTW, where's the canonical source for ns_exec?
> 
> It is here: git://git.sr71.net/~hallyn/cr_tests.git

Thanks a lot.

> So I guess this is the first time we are trying this scenario out (i.e.
> it may not be a regression so we can't necessarily use git-bisect).
> 
> I am not too familiar with libvirt implementation, but do you think it
> is possible to repro this using ns_exec and fewer/minimal namespaces ?

I didn't try that, but regarding devpts the libvirt code does basically:

unshare(CLONE_NS)
mount --make-slave /
mount devpts /dev/pts -t devpts 

clone(all the namespace options including CLONE_NEWNS)
child:
 mount --move /the/above/dev/pts /dev/pts
 set up everything else

As the point of the bug is opening a pts when its other end is already
gone, I'm not sure we have to even do anything in the parent (maybe
open/close the ptm).

Will have a look today.

> We should be able to get a cross-namespace pty using ns_exec, but am not
> sure the set of namespaces need to be cloned -  would it be sufficient if
> we clone pid and mount namespaces and remount /dev/pts in child container ?

As my testcase doesn't know nor care about its pid, quite possibly we
would only need mount namespaces.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Tue, Jul 14, 2009 at 09:47:44PM -0700, Sukadev Bhattiprolu wrote:
> I am not too familiar with libvirt implementation, but do you think it
> is possible to repro this using ns_exec and fewer/minimal namespaces ?

Just run the program below. 2.6.29 oopses when the child tries to open
/dev/console. Possibly can be stripped down even further but I think
it's fairly minimal anyway.

BTW, I guess the patch should set ERR_PTR(-EIO) or something, instead
of NULL, am I right?

Best regards,
 Grzegorz Nosek

/*------------- cut ------------*/
#define _GNU_SOURCE
#include <fcntl.h>
#include <sched.h>
#include <stdlib.h>
#include <sys/mount.h>
#include <sys/signal.h>
#include <unistd.h>

#include <linux/fs.h>

void dummy(int sig)
{
}

static int child(void *unused)
{
	signal(SIGINT, dummy);
	signal(SIGHUP, dummy);
	pause(); /* cheesy synchronisation to wait for /dev/pts/0 to appear */

	mount("/dev/pts/0", "/dev/console", NULL, MS_BIND, NULL);
	sleep(2);

	open("/dev/console", O_RDWR);
	dup(0);
	dup(0);
	write(1, "Hello world!\n", sizeof("Hello world!\n")-1);
	return 0;
}

int main(void)
{
	pid_t pid;
	char *stack;
	int fd;

	stack = malloc(16384);
	pid = clone(child, stack+16384, CLONE_NEWNS|SIGCHLD, NULL);

	fd = open("/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK);
	unlockpt(fd);
	grantpt(fd);

	kill(pid, SIGHUP);
	sleep(1);
	return 0; /* exit before child opens /dev/console */
}

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| On Tue, Jul 14, 2009 at 09:47:44PM -0700, Sukadev Bhattiprolu wrote:
| > I am not too familiar with libvirt implementation, but do you think it
| > is possible to repro this using ns_exec and fewer/minimal namespaces ?
| 

I have been trying to setup libvirt+lxc and am running into unrelated
problems.  That said,

| Just run the program below.

I defined this tty-bug program the container-init in my lxc.xml file

        <os>
                <type>exe</type>
                <init>/root/lxc/tty-bug</init>
        </os>

and tried running:

	virsh --connect lxc:/// start lxc-vm2

Is that what you mean by "just run" :-) ?

If so, I have to first resolve my unrelated problem with libvirtd+lxc.

Running "ns_exec -cpuim ./tty-bug", I dont get the failure with 2.6.31-rc3
on a Fedora 11 KVM.

Sukadev

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

Hi,

On sob, lip 18, 2009 at 01:52:44 -0700, Sukadev Bhattiprolu wrote:
> 	virsh --connect lxc:/// start lxc-vm2
> 
> Is that what you mean by "just run" :-) ?
> If so, I have to first resolve my unrelated problem with libvirtd+lxc.
> 
> Running "ns_exec -cpuim ./tty-bug", I dont get the failure with 2.6.31-rc3
> on a Fedora 11 KVM.

I get the oops by simply running the program, without libvirtd,
lxc-tools or ns_exec (it sets up a mount namespace itself). Are you
running with slub debugging (poisoning) enabled? The bug causes
access to freed memory, so otherwise often nothing will happen
(the memory won't have been overwritten yet).

OTOH, I haven't tried 2.6.31-rc3 either (but it doesn't look like fixed
there too).

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| Hi,
| 
| On sob, lip 18, 2009 at 01:52:44 -0700, Sukadev Bhattiprolu wrote:
| > 	virsh --connect lxc:/// start lxc-vm2
| > 
| > Is that what you mean by "just run" :-) ?
| > If so, I have to first resolve my unrelated problem with libvirtd+lxc.
| > 
| > Running "ns_exec -cpuim ./tty-bug", I dont get the failure with 2.6.31-rc3
| > on a Fedora 11 KVM.
| 
| I get the oops by simply running the program, without libvirtd,
| lxc-tools or ns_exec (it sets up a mount namespace itself). Are you
| running with slub debugging (poisoning) enabled? 

I set 
	CONFIG_SLUB_DEBUG=y
	CONFIG_SLUB=y
	CONFIG_SLUB_DEBUG_ON=y
	# CONFIG_SLUB_STATS is not set

and tried 2.6.29, 2.6.31-rc3 and linux-mmotm from July 13, but have
not been able to repro either on an i386 machine or on a KVM guest.

I run your program ./tty-bug in a tight loop. I will try to run the
program overnight in a loop. 

Given that your program does not depend on NET_NS, can you see if you
can repro on 2.6.28 and see if we can bisect this problem ?

| The bug causes
| access to freed memory, so otherwise often nothing will happen
| (the memory won't have been overwritten yet).
| 
| OTOH, I haven't tried 2.6.31-rc3 either (but it doesn't look like fixed
| there too).
| 
| Best regards,
|  Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On wto, lip 21, 2009 at 11:41:20 -0700, Sukadev Bhattiprolu wrote:
> I set 
> 	CONFIG_SLUB_DEBUG=y
> 	CONFIG_SLUB=y
> 	CONFIG_SLUB_DEBUG_ON=y
> 	# CONFIG_SLUB_STATS is not set
> 
> and tried 2.6.29, 2.6.31-rc3 and linux-mmotm from July 13, but have
> not been able to repro either on an i386 machine or on a KVM guest.
> 
> I run your program ./tty-bug in a tight loop. I will try to run the
> program overnight in a loop. 
> 
> Given that your program does not depend on NET_NS, can you see if you
> can repro on 2.6.28 and see if we can bisect this problem ?

Immediate crash. I tried 2.6.18-something (Debian etch kernel) that I
had lying around on the VM. The result:

idr_remove called for id=0 which is not allocated.
 [<c01b7abc>] idr_remove+0xd4/0x137
 [<c01fa871>] release_mem+0x1d5/0x1e1
 [<c01fb4ec>] release_dev+0x5d6/0x5ee
 [<c011669e>] __wake_up+0x2a/0x3d
 [<c01f9e1f>] tty_ldisc_enable+0x1f/0x21
 [<c01fabf5>] init_dev+0x378/0x49f
 [<c01fd2e4>] tty_open+0x2a9/0x2e8
 [<c0161899>] chrdev_open+0x126/0x141
 [<c0161773>] chrdev_open+0x0/0x141
 [<c0158b65>] __dentry_open+0xc8/0x1ac
 [<c0158cad>] nameidata_to_filp+0x19/0x28
 [<c0158ce7>] do_filp_open+0x2b/0x31
 [<c027fddd>] do_nanosleep+0x43/0x6a
 [<c0125f96>] do_sigaction+0x99/0x156
 [<c0158d2b>] do_sys_open+0x3e/0xb3
 [<c0158dcd>] sys_open+0x16/0x18
 [<c0102c7b>] syscall_call+0x7/0xb

(on the bright side, the machine is still usable afterwards).

However, 2.6.26 (both mine and Debian) survives the test so it may indeed
be a recent regression (was it broken again after fixing sometime
between .18 and .26?)

Bisecting...

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| On wto, lip 21, 2009 at 11:41:20 -0700, Sukadev Bhattiprolu wrote:
| > I set 
| > 	CONFIG_SLUB_DEBUG=y
| > 	CONFIG_SLUB=y
| > 	CONFIG_SLUB_DEBUG_ON=y
| > 	# CONFIG_SLUB_STATS is not set
| > 
| > and tried 2.6.29, 2.6.31-rc3 and linux-mmotm from July 13, but have
| > not been able to repro either on an i386 machine or on a KVM guest.
| > 
| > I run your program ./tty-bug in a tight loop. I will try to run the
| > program overnight in a loop. 
| > 
| > Given that your program does not depend on NET_NS, can you see if you
| > can repro on 2.6.28 and see if we can bisect this problem ?
| 
| Immediate crash. I tried 2.6.18-something (Debian etch kernel) that I
| had lying around on the VM. The result:

Interesting.

Attaching test program and Ccing Peter Anvin for any insights.

| 
| idr_remove called for id=0 which is not allocated.
|  [<c01b7abc>] idr_remove+0xd4/0x137
|  [<c01fa871>] release_mem+0x1d5/0x1e1
|  [<c01fb4ec>] release_dev+0x5d6/0x5ee
|  [<c011669e>] __wake_up+0x2a/0x3d
|  [<c01f9e1f>] tty_ldisc_enable+0x1f/0x21
|  [<c01fabf5>] init_dev+0x378/0x49f
|  [<c01fd2e4>] tty_open+0x2a9/0x2e8
|  [<c0161899>] chrdev_open+0x126/0x141
|  [<c0161773>] chrdev_open+0x0/0x141
|  [<c0158b65>] __dentry_open+0xc8/0x1ac
|  [<c0158cad>] nameidata_to_filp+0x19/0x28
|  [<c0158ce7>] do_filp_open+0x2b/0x31
|  [<c027fddd>] do_nanosleep+0x43/0x6a
|  [<c0125f96>] do_sigaction+0x99/0x156
|  [<c0158d2b>] do_sys_open+0x3e/0xb3
|  [<c0158dcd>] sys_open+0x16/0x18
|  [<c0102c7b>] syscall_call+0x7/0xb
| 
| (on the bright side, the machine is still usable afterwards).
| 
| However, 2.6.26 (both mine and Debian) survives the test so it may indeed
| be a recent regression (was it broken again after fixing sometime
| between .18 and .26?)
| 
| Bisecting...
| 
| Best regards,
|  Grzegorz Nosek

[-- Attachment #2: tty-bug.c --]
[-- Type: text/x-csrc, Size: 848 bytes --]

#define _GNU_SOURCE
#include <fcntl.h>
#include <sched.h>
#include <stdlib.h>
#include <sys/mount.h>
#include <sys/signal.h>
#include <unistd.h>

#include <linux/fs.h>

void dummy(int sig)
{
}

static int child(void *unused)
{
	signal(SIGINT, dummy);
	signal(SIGHUP, dummy);
	pause(); /* cheesy synchronisation to wait for /dev/pts/0 to appear */

	mount("/dev/pts/0", "/dev/console", NULL, MS_BIND, NULL);
	sleep(2);

	open("/dev/console", O_RDWR);
	dup(0);
	dup(0);
	write(1, "Hello world!\n", sizeof("Hello world!\n")-1);
	return 0;
}

int main(void)
{
	pid_t pid;
	char *stack;
	int fd;

	stack = malloc(16384);
	pid = clone(child, stack+16384, CLONE_NEWNS|SIGCHLD, NULL);

	fd = open("/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK);
	unlockpt(fd);
	grantpt(fd);

	kill(pid, SIGHUP);
	sleep(1);
	return 0; /* exit before child opens /dev/console */
}

[-- Attachment #3: Type: text/plain, Size: 206 bytes --]

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[H. Peter Anvin]

On 07/22/2009 06:27 PM, Sukadev Bhattiprolu wrote:
> | 
> | Immediate crash. I tried 2.6.18-something (Debian etch kernel) that I
> | had lying around on the VM. The result:
> 
> Interesting.
> 
> Attaching test program and Ccing Peter Anvin for any insights.
> 
> | idr_remove called for id=0 which is not allocated.
> |  [<c01b7abc>] idr_remove+0xd4/0x137
> |  [<c01fa871>] release_mem+0x1d5/0x1e1
> |  [<c01fb4ec>] release_dev+0x5d6/0x5ee
> |  [<c011669e>] __wake_up+0x2a/0x3d
> |  [<c01f9e1f>] tty_ldisc_enable+0x1f/0x21
> |  [<c01fabf5>] init_dev+0x378/0x49f
> |  [<c01fd2e4>] tty_open+0x2a9/0x2e8
> |  [<c0161899>] chrdev_open+0x126/0x141
> |  [<c0161773>] chrdev_open+0x0/0x141
> |  [<c0158b65>] __dentry_open+0xc8/0x1ac
> |  [<c0158cad>] nameidata_to_filp+0x19/0x28
> |  [<c0158ce7>] do_filp_open+0x2b/0x31
> |  [<c027fddd>] do_nanosleep+0x43/0x6a
> |  [<c0125f96>] do_sigaction+0x99/0x156
> |  [<c0158d2b>] do_sys_open+0x3e/0xb3
> |  [<c0158dcd>] sys_open+0x16/0x18
> |  [<c0102c7b>] syscall_call+0x7/0xb
> | 
> | (on the bright side, the machine is still usable afterwards).
> | 
> | However, 2.6.26 (both mine and Debian) survives the test so it may indeed
> | be a recent regression (was it broken again after fixing sometime
> | between .18 and .26?)
> | 
> | Bisecting...

Interesting... I have to say I'm more than a bit surprised that you can
mount a filesystem on top of a character device node at all, but there
isn't really a fundamental reason why you couldn't do it, so...

I am assuming that what causes the problem is that you have found a way
(vfsmount) to hold the pts device node busy which doesn't involve the
tty subsystem.  This isn't inherently a problem, but it does have
implications for freeing: in particular, the pts node cannot be removed
until the vfsmount is gone, *and* the device number cannot be reclaimed.
 It sounds like it's the latter piece which causes problems.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On śro, lip 22, 2009 at 06:48:55 -0700, H. Peter Anvin wrote:
> > | Bisecting...

2.6.27 is good, 2.6.28 is bad. Some 600-odd revisions still to go (will
continue in the evening).

> Interesting... I have to say I'm more than a bit surprised that you can
> mount a filesystem on top of a character device node at all, but there
> isn't really a fundamental reason why you couldn't do it, so...

Actually it's mounting a device node over a device node. I was really
surprised when I learnt that you can mount --bind other things than
directories but thought it could be the closest a container can get to a
real console. So mounting /dev/pts/0 over /dev/console seemed quite
natural to me.

> I am assuming that what causes the problem is that you have found a way
> (vfsmount) to hold the pts device node busy which doesn't involve the
> tty subsystem.  This isn't inherently a problem, but it does have
> implications for freeing: in particular, the pts node cannot be removed
> until the vfsmount is gone, *and* the device number cannot be reclaimed.
>  It sounds like it's the latter piece which causes problems.

Upthread I posted a patch[1] that makes tty_driver_lookup_tty() return
NULL (later changed to ERR_PTR(-EIO) and verified it works) in such a
case. If the inode can (as you say) legally outlive its matching tty_struct,
I guess the patch should go in. If it cannot, well, the patch is bogus.

Best regards,
 Grzegorz Nosek

[1] https://lists.linux-foundation.org/pipermail/containers/2009-July/019141.html
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [lxc-devel] BUG in tty_open when using containers and ptrace

[Daniel Lezcano]

Grzegorz Nosek wrote:
> On śro, lip 22, 2009 at 06:48:55 -0700, H. Peter Anvin wrote:
>   
>>> | Bisecting...
>>>       
>
> 2.6.27 is good, 2.6.28 is bad. Some 600-odd revisions still to go (will
> continue in the evening).
>
>   
>> Interesting... I have to say I'm more than a bit surprised that you can
>> mount a filesystem on top of a character device node at all, but there
>> isn't really a fundamental reason why you couldn't do it, so...
>>     
>
> Actually it's mounting a device node over a device node. I was really
> surprised when I learnt that you can mount --bind other things than
> directories but thought it could be the closest a container can get to a
> real console. So mounting /dev/pts/0 over /dev/console seemed quite
> natural to me.
>   
Absolutely and this is how is done the console / tty management in liblxc.
Bind mounting is very powerful when it is used with the mount namespace 
because you can have multiple processes in different mount namespaces 
accessing the same path but with different real locations : a perfect 
feature for the containers :)

  -- Daniel
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On śro, lip 22, 2009 at 06:48:55 -0700, H. Peter Anvin wrote:
> > | Bisecting...

OK, the first bad commit is:
commit 4a2b5fddd53b80efcb3266ee36e23b8de28e761a
Author: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Date:   Mon Oct 13 10:42:49 2008 +0100

    Move tty lookup/reopen to caller

    Move tty_driver_lookup_tty() and tty_reopen() from tty_init_dev()
    into tty_open() (one of the two callers of tty_init_dev()).  These
    calls are not really required in ptmx_open(), the other caller,
    since ptmx_open() would be setting up a new tty.

    Changelog[v2]:
        - remove the lookup and reopen calls from ptmx_open
        - merge with recent changes to ttydev tree

    Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
    Signed-off-by: Alan Cox <alan@redhat.com>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

However, with HEAD there the behaviour is slightly different as the
kernel does not crash (loudly) and the open succeeds but silently
leaves the tty layer locked, so it's just as bad.

The oops itself is introduced by:
commit 527b3e4773628b30d03323a2cb5fb0d84441990f
Author: Sukadev Bhattiprolu <sukadev@us.ibm.com>
Date:   Mon Oct 13 10:43:08 2008 +0100

    Simplify devpts_get_tty()

    As pointed out by H. Peter Anvin, since the inode for the pty is known,
    we don't need to look it up.

    Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
    Signed-off-by: Alan Cox <alan@redhat.com>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>


Bisection log:

git-bisect start
# good: [3fa8749e584b55f1180411ab1b51117190bac1e5] Linux 2.6.27
git-bisect good 3fa8749e584b55f1180411ab1b51117190bac1e5
# bad: [4a6908a3a050aacc9c3a2f36b276b46c0629ad91] Linux 2.6.28
git-bisect bad 4a6908a3a050aacc9c3a2f36b276b46c0629ad91
# bad: [c813b4e16ead3c3df98ac84419d4df2adf33fe01] Merge git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core-2.6
git-bisect bad c813b4e16ead3c3df98ac84419d4df2adf33fe01
# good: [8fa0b315fc0c1a414da1371f1fc39523a657c192] decnet: Fix compiler warning in dn_dev.c
git-bisect good 8fa0b315fc0c1a414da1371f1fc39523a657c192
# bad: [9c6102d446985bca9c426cb2d9b478ed21d2b024] ata: Add documentation for hard disk shock protection interface (v3)
git-bisect bad 9c6102d446985bca9c426cb2d9b478ed21d2b024
# good: [1a2217a9516b134e0a0e54cb4629e1e075d97b17] Merge git://git.kernel.org/pub/scm/linux/kernel/git/hskinnemoen/avr32-2.6
git-bisect good 1a2217a9516b134e0a0e54cb4629e1e075d97b17
# good: [c00193f9f09f9b852249a66391985f585d066084] Merge branches 'oprofile-v2' and 'timers/hpet' into x86/core-v4
git-bisect good c00193f9f09f9b852249a66391985f585d066084
# bad: [20272c8994cf1e1f8ed745a2ea161dd9ad3889f2] Merge branch 'proc' of git://git.kernel.org/pub/scm/linux/kernel/git/adobriyan/proc
git-bisect bad 20272c8994cf1e1f8ed745a2ea161dd9ad3889f2
# bad: [8dff04ea316125639120c0a565ce0346b892fef7] pty: simplify unix98 allocation
git-bisect bad 8dff04ea316125639120c0a565ce0346b892fef7
# good: [9e48565d217a8a96cc7577308ad41e9e4b806a62] tty: Split tty_port into its own file
git-bisect good 9e48565d217a8a96cc7577308ad41e9e4b806a62
# good: [dbda4c0b97b18fd59b3964548361b4f92357f730] tty: Fix abusers of current->sighand->tty
git-bisect good dbda4c0b97b18fd59b3964548361b4f92357f730
# good: [73ec06fc5f5c8e1097a7a4a4ab2d7c6c3a007e81] tty: Finish fixing up the init_dev interface to use ERR_PTR
git-bisect good 73ec06fc5f5c8e1097a7a4a4ab2d7c6c3a007e81
# bad: [527b3e4773628b30d03323a2cb5fb0d84441990f] Simplify devpts_get_tty()
git-bisect bad 527b3e4773628b30d03323a2cb5fb0d84441990f
# bad: [4a2b5fddd53b80efcb3266ee36e23b8de28e761a] Move tty lookup/reopen to caller
git-bisect bad 4a2b5fddd53b80efcb3266ee36e23b8de28e761a
# good: [bf970ee46e0fb363c8df4393229121d54330a98e] tty: extract the pty init time special cases
git-bisect good bf970ee46e0fb363c8df4393229121d54330a98e


_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root@localdomain.pl] wrote:
| On śro, lip 22, 2009 at 06:48:55 -0700, H. Peter Anvin wrote:
| > > | Bisecting...
| 
| OK, the first bad commit is:
| commit 4a2b5fddd53b80efcb3266ee36e23b8de28e761a
| Author: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| Date:   Mon Oct 13 10:42:49 2008 +0100
| 
|     Move tty lookup/reopen to caller
| 
|     Move tty_driver_lookup_tty() and tty_reopen() from tty_init_dev()
|     into tty_open() (one of the two callers of tty_init_dev()).  These
|     calls are not really required in ptmx_open(), the other caller,
|     since ptmx_open() would be setting up a new tty.
| 
|     Changelog[v2]:
|         - remove the lookup and reopen calls from ptmx_open
|         - merge with recent changes to ttydev tree
| 
|     Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
|     Signed-off-by: Alan Cox <alan@redhat.com>
|     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
| 
| However, with HEAD there the behaviour is slightly different as the
| kernel does not crash (loudly) and the open succeeds but silently
| leaves the tty layer locked, so it's just as bad.
| 

Yes, I think this was fixed by 808ffa3d302257b9dc37b1412c1fcdf976fcddac

| The oops itself is introduced by:
| commit 527b3e4773628b30d03323a2cb5fb0d84441990f
| Author: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| Date:   Mon Oct 13 10:43:08 2008 +0100
| 
|     Simplify devpts_get_tty()
| 
|     As pointed out by H. Peter Anvin, since the inode for the pty is known,
|     we don't need to look it up.
| 
|     Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
|     Signed-off-by: Alan Cox <alan@redhat.com>
|     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Maybe before returning the tty, we need to take an extra reference on
the dentry.

Sukadev

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On czw, lip 23, 2009 at 12:32:45 -0700, Sukadev Bhattiprolu wrote:
> | OK, the first bad commit is:
> | commit 4a2b5fddd53b80efcb3266ee36e23b8de28e761a
> 
> Yes, I think this was fixed by 808ffa3d302257b9dc37b1412c1fcdf976fcddac

Any other tests I can do that will be helpful? E.g. 2.6.27 + 808ffa?

> | The oops itself is introduced by:
> | commit 527b3e4773628b30d03323a2cb5fb0d84441990f
> | Author: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> | Date:   Mon Oct 13 10:43:08 2008 +0100
> | 
> |     Simplify devpts_get_tty()
> | 
> |     As pointed out by H. Peter Anvin, since the inode for the pty is known,
> |     we don't need to look it up.
> | 
> |     Signed-off-by: Sukadev Bhattiprolu <sukadev-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> |     Signed-off-by: Alan Cox <alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> |     Signed-off-by: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> 
> Maybe before returning the tty, we need to take an extra reference on
> the dentry.

I tried that and while it did not oops, I wasn't sure where to dput it
back, so it leaked like a sieve. I'm probably missing something obvious
but I couldn't find a function whose calls balanced calls to
devpts_get_tty.

BTW, what would the semantics be? I.e. what can we do with a pty that has
its master side long gone?

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[H. Peter Anvin]

Grzegorz Nosek wrote:
> 
> I tried that and while it did not oops, I wasn't sure where to dput it
> back, so it leaked like a sieve. I'm probably missing something obvious
> but I couldn't find a function whose calls balanced calls to
> devpts_get_tty.
> 
> BTW, what would the semantics be? I.e. what can we do with a pty that has
> its master side long gone?
> 

Nothing, but as long as something is keeping the pts file entry open, it
 should not be garbage-collected.

	-hpa

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

On Thu, Jul 23, 2009 at 03:11:50PM -0700, H. Peter Anvin wrote:
> Nothing, but as long as something is keeping the pts file entry open, it
>  should not be garbage-collected.

The pts is opened after the parent (holding the master side) exits, so the
ptm is not going away from under us while we have the pts open -- it's no
longer there at open() time.

The lifetime rules and relations for master tty_struct, slave tty_struct
and the corresponding devpts inode are still not completely clear to me
(what is holding what alive etc.), so I want to be 100% sure we're on the
same page.

In this case it looks like the ttys are gone but the inode still keeps a
pointer to the pts tty_struct. So we can either:
 1. overwrite the pointer with -EIO or something
 2. extend the life of the ttys to match the inode

As the end result from userspace perspective is the same (we have to
error out when opening the pts anyway -- as you implied above), I'd go for
the simpler (1).

BTW, I was mistaken about experimenting with the dentry refcount -- I
did it the other way around (tty_kref_get while creating the inode
IIRC). Sorry for the confusion.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| On Thu, Jul 23, 2009 at 03:11:50PM -0700, H. Peter Anvin wrote:
| > Nothing, but as long as something is keeping the pts file entry open, it
| >  should not be garbage-collected.
| 
| The pts is opened after the parent (holding the master side) exits, so the
| ptm is not going away from under us while we have the pts open -- it's no
| longer there at open() time.
| 
| The lifetime rules and relations for master tty_struct, slave tty_struct
| and the corresponding devpts inode are still not completely clear to me
| (what is holding what alive etc.), so I want to be 100% sure we're on the
| same page.
| 
| In this case it looks like the ttys are gone but the inode still keeps a
| pointer to the pts tty_struct. So we can either:
|  1. overwrite the pointer with -EIO or something
|  2. extend the life of the ttys to match the inode
| 
| As the end result from userspace perspective is the same (we have to
| error out when opening the pts anyway -- as you implied above), I'd go for
| the simpler (1).
| 
| BTW, I was mistaken about experimenting with the dentry refcount -- I
| did it the other way around (tty_kref_get while creating the inode
| IIRC). Sorry for the confusion.

Well, I made some progress on this. In an earlier mail you mentioned
that your test case "survives" in kernels without the bad commit.

With the following patch the kernel seems to _survive_ but not sure if
"surviving" meets your real needs (i.e does the pty continue to function
as the console for the container) ? 

The way I understand, the child process does not get an open-file reference
to the pty. So when the parent closes the pty, devpts_pty_kill() unhashes
the dentry (although the bind mount() in the child has a reference count
on dentry).

Sukadev

---

Index: linux-2.6.30/fs/devpts/inode.c
===================================================================
--- linux-2.6.30.orig/fs/devpts/inode.c 2009-07-30 10:48:17.000000000 -0400
+++ linux-2.6.30/fs/devpts/inode.c      2009-07-30 10:58:37.000000000 -0400
@@ -519,8 +519,14 @@

 struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
 {
+       struct dentry *dentry;
+
        BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));

+       dentry = d_find_alias(pts_inode);
+       if (IS_ERR(dentry))
+               return NULL;
+
        if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
                return (struct tty_struct *)pts_inode->i_private;
        return NULL;

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Sukadev Bhattiprolu [sukadev-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org] wrote:
| Index: linux-2.6.30/fs/devpts/inode.c
| ===================================================================
| --- linux-2.6.30.orig/fs/devpts/inode.c 2009-07-30 10:48:17.000000000 -0400
| +++ linux-2.6.30/fs/devpts/inode.c      2009-07-30 10:58:37.000000000 -0400
| @@ -519,8 +519,14 @@
| 
|  struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
|  {
| +       struct dentry *dentry;
| +
|         BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));
| 
| +       dentry = d_find_alias(pts_inode);
| +       if (IS_ERR(dentry))

Sorry, this should be:

	if (!dentry)

| +               return NULL;
| +
|         if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
|                 return (struct tty_struct *)pts_inode->i_private;
|         return NULL;
| 
| _______________________________________________
| Containers mailing list
| Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
| https://lists.linux-foundation.org/mailman/listinfo/containers

Re: BUG in tty_open when using containers and ptrace

[Grzegorz Nosek]

Hi,

Dnia 2009-07-30, czw o godzinie 11:06 -0700, Sukadev Bhattiprolu pisze:
> Well, I made some progress on this. In an earlier mail you mentioned
> that your test case "survives" in kernels without the bad commit.
> 
> With the following patch the kernel seems to _survive_ but not sure if
> "surviving" meets your real needs (i.e does the pty continue to function
> as the console for the container) ? 

Both my patch and yours make the console open fail and I'm not sure we
can do much better. I don't think this matters (too much) either as the
supervisor (libvirt_lxc) exits only on container shutdown, so there's
nothing much we can do to the container anyway.

During normal operation the console works just like before. However,
(but this is not related to the patch at all) I can't get it to work
predictably, let alone usefully (mounting the console from the libvirtd
config file silently fails, leaving the console at major:minor 5:1 and
messing with the host's console; mounting it from inside the container
apparently races with something and sometimes fails with /dev/pts/0
missing and anyway exposes the host's console to init). Will see
tomorrow.

> The way I understand, the child process does not get an open-file reference
> to the pty. So when the parent closes the pty, devpts_pty_kill() unhashes
> the dentry (although the bind mount() in the child has a reference count
> on dentry).

Sounds plausible and your patch communicates the idea quite clearly.

Best regards,
 Grzegorz Nosek

Re: BUG in tty_open when using containers and ptrace

[Sukadev Bhattiprolu]

Grzegorz Nosek [root-AfQBxy1nhrQ00sYp1HPQUA@public.gmane.org] wrote:
| Hi,
| 
| Dnia 2009-07-30, czw o godzinie 11:06 -0700, Sukadev Bhattiprolu pisze:
| > Well, I made some progress on this. In an earlier mail you mentioned
| > that your test case "survives" in kernels without the bad commit.
| > 
| > With the following patch the kernel seems to _survive_ but not sure if
| > "surviving" meets your real needs (i.e does the pty continue to function
| > as the console for the container) ? 
| 
| Both my patch and yours make the console open fail and I'm not sure we
| can do much better. I don't think this matters (too much) either as the
| supervisor (libvirt_lxc) exits only on container shutdown, so there's
| nothing much we can do to the container anyway.

Ok.  Hope you had some luck with the console and libvirtd.

FWIW, should we push this patch upstream ? It would at least prevent
the kernel crash and be consistent with the behavior before newptsinstance
changes ?

Peter, what do you think about this patch ?

---
Index: linux-2.6.30/fs/devpts/inode.c
===================================================================
--- linux-2.6.30.orig/fs/devpts/inode.c 2009-07-30 10:48:17.000000000 -0400
+++ linux-2.6.30/fs/devpts/inode.c      2009-08-04 11:43:51.000000000 -0400
@@ -519,11 +519,22 @@

 struct tty_struct *devpts_get_tty(struct inode *pts_inode, int number)
 {
+       struct dentry *dentry;
+       struct tty_struct *tty;
+
        BUG_ON(pts_inode->i_rdev == MKDEV(TTYAUX_MAJOR, PTMX_MINOR));

+       dentry = d_find_alias(pts_inode);
+       if (!dentry)
+               return NULL;
+
+       tty = NULL;
        if (pts_inode->i_sb->s_magic == DEVPTS_SUPER_MAGIC)
-               return (struct tty_struct *)pts_inode->i_private;
-       return NULL;
+               tty = (struct tty_struct *)pts_inode->i_private;
+
+       dput(dentry);
+
+       return tty;
 }

 void devpts_pty_kill(struct tty_struct *tty)