Re: hvc_console: Don't access hvc_task if not initialised
On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah amit.s...@redhat.com wrote: On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote: Care to either create this patch, or resend your original one, if you want it applied? Rusty has the other one queued. I pinged him about status. It's merged, but I don't think it had the requisite cc:stable, did it? Thanks, Rusty. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote: On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah amit.s...@redhat.com wrote: On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote: Care to either create this patch, or resend your original one, if you want it applied? Rusty has the other one queued. I pinged him about status. It's merged, but I don't think it had the requisite cc:stable, did it? No, because at the point of submitting I didn't know it would solve this bug as well. Greg, can you pick afa2689e19073cd2e762d0f2c1358fab1ab9f18c for stable? Thanks, Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On Wed, Apr 27, 2011 at 12:01:47PM +0530, Amit Shah wrote: On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote: On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah amit.s...@redhat.com wrote: On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote: Care to either create this patch, or resend your original one, if you want it applied? Rusty has the other one queued. I pinged him about status. It's merged, but I don't think it had the requisite cc:stable, did it? No, because at the point of submitting I didn't know it would solve this bug as well. Greg, can you pick afa2689e19073cd2e762d0f2c1358fab1ab9f18c for stable? Yes, but in the future, this is NOT the way to get patches into the stable tree. Please read Documentation/stable_kernel_rules.txt for how to do it properly. thanks, greg k-h ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On (Wed) 27 Apr 2011 [17:09:34], Greg KH wrote: On Wed, Apr 27, 2011 at 12:01:47PM +0530, Amit Shah wrote: On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote: On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah amit.s...@redhat.com wrote: On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote: Care to either create this patch, or resend your original one, if you want it applied? Rusty has the other one queued. I pinged him about status. It's merged, but I don't think it had the requisite cc:stable, did it? No, because at the point of submitting I didn't know it would solve this bug as well. Greg, can you pick afa2689e19073cd2e762d0f2c1358fab1ab9f18c for stable? Yes, but in the future, this is NOT the way to get patches into the stable tree. Please read Documentation/stable_kernel_rules.txt for how to do it properly. Got it, thanks! Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote: On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote: On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote: On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote: hvc_open() can be called without having any backing device. This results in a call to hvc_kick() which calls wake_up_process on a NULL pointer. How is hvc_open called without a hvc_driver registered to the tty layer? This gets reproduced in a couple of scenarios, I'm trying to get more information. OK - I finally could reproduce myself, albiet it's a panic in hvc_open, not the one mentioned earlier. hvc_console is built into the kernel and virtio_console is a module. This sequence triggers a panic: - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 - rmmod virtio_console - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 A patch that I had sent previously, to hvc_remove() a port when the associated virtio_console port gets unplugged, fixes this panic. Stricter checking in hvc_open(), as you mentioned, will solve the other one as well. Thanks! Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On Wed, Apr 20, 2011 at 06:03:30PM +0530, Amit Shah wrote: On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote: On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote: On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote: On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote: hvc_open() can be called without having any backing device. This results in a call to hvc_kick() which calls wake_up_process on a NULL pointer. How is hvc_open called without a hvc_driver registered to the tty layer? This gets reproduced in a couple of scenarios, I'm trying to get more information. OK - I finally could reproduce myself, albiet it's a panic in hvc_open, not the one mentioned earlier. hvc_console is built into the kernel and virtio_console is a module. This sequence triggers a panic: - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 - rmmod virtio_console - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 A patch that I had sent previously, to hvc_remove() a port when the associated virtio_console port gets unplugged, fixes this panic. Stricter checking in hvc_open(), as you mentioned, will solve the other one as well. Care to either create this patch, or resend your original one, if you want it applied? thanks, greg k-h ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote: On Wed, Apr 20, 2011 at 06:03:30PM +0530, Amit Shah wrote: On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote: On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote: On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote: On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote: hvc_open() can be called without having any backing device. This results in a call to hvc_kick() which calls wake_up_process on a NULL pointer. How is hvc_open called without a hvc_driver registered to the tty layer? This gets reproduced in a couple of scenarios, I'm trying to get more information. OK - I finally could reproduce myself, albiet it's a panic in hvc_open, not the one mentioned earlier. hvc_console is built into the kernel and virtio_console is a module. This sequence triggers a panic: - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 - rmmod virtio_console - modprobe virtio_console - agetty /dev/hvc0 9600 vt100 A patch that I had sent previously, to hvc_remove() a port when the associated virtio_console port gets unplugged, fixes this panic. Stricter checking in hvc_open(), as you mentioned, will solve the other one as well. Care to either create this patch, or resend your original one, if you want it applied? Rusty has the other one queued. I pinged him about status. Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote: On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote: On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote: hvc_open() can be called without having any backing device. This results in a call to hvc_kick() which calls wake_up_process on a NULL pointer. How is hvc_open called without a hvc_driver registered to the tty layer? This gets reproduced in a couple of scenarios, I'm trying to get more information. Ensure hvc is initialised by checking for a non-NULL hvc_task before waking up the hvc thread. No if the task is missing the subsystem is really stuck. Put a check in open and refuse to open. Just wanted to add emphasis. Making hvc_kick a do nothing is not acceptable. This was found by an autotest run for virtio_console without having a console backend. Are you sure? the virtio_console module was loaded. stack trace please Yes, should have included that: BUG: unable to handle kernel NULL pointer dereference at 0008 IP: [8104fef2] task_rq_lock+0x42/0xa0 PGD 1d66a067 PUD 1d598067 PMD 0 Oops: [#1] SMP last sysfs file: /sys/devices/pci:00/:00:04.0/virtio1/virtio-ports/vport0p0/name CPU 0 Modules linked in: virtio_console virtio_net i2c_piix4 i2c_core ext4 mbcache jbd2 sr_mod cdrom virtio_blk pata_acpi ata_generic ata_piix virtio_pci virtio_ring virtio dm_mod [last unloaded: scsi_wait_scan] Modules linked in: virtio_console virtio_net i2c_piix4 i2c_core ext4 mbcache jbd2 sr_mod cdrom virtio_blk pata_acpi ata_generic ata_piix virtio_pci virtio_ring virtio dm_mod [last unloaded: scsi_wait_scan] Pid: 672, comm: console_check Not tainted 2.6.32-125.el6.x86_64 #1 KVM So 2.6.32-125.el6.x86_64, not mainline. Please check for any patches affecting hvc_task and/or hvc_driver. RIP: 0010:[8104fef2] [8104fef2] task_rq_lock+0x42/0xa0 RSP: 0018:880017cbbb98 EFLAGS: 00010082 RAX: 0282 RBX: 00015f40 RCX: 0002 RDX: 0282 RSI: 880017cbbbf0 RDI: RBP: 880017c8 R08: 88001bf39208 R09: 88001d67a000 R10: 0001 R11: R12: R13: 880017cbbbf0 R14: R15: 000f FS: 7f3592683700() GS:88000200() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 0008 CR3: 1d595000 CR4: 06f0 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400 so seems to be x86_64 Process console_check (pid: 672, threadinfo 880017cba000, task 88001f5fe100) Stack: 88001bf39000 0 880017cbbc28 8105c74c 88001d5966a0 8800 0 880017cbbbe8 813081c1 880017cbbc28 0282 Call Trace: [8105c74c] try_to_wake_up+0x3c/0x400 [813081c1] ? tty_ldisc_enable+0x31/0x40 [8105cb65] wake_up_process+0x15/0x20 [8131a10f] hvc_kick+0x1f/0x30 [8131a58a] hvc_open+0xca/0x120 [81302ba1] tty_open+0x211/0x510 [811754b5] chrdev_open+0x125/0x230 [81175390] ? chrdev_open+0x0/0x230 [8116e70a] __dentry_open+0x10a/0x360 [8120d022] ? selinux_inode_permission+0x72/0xb0 [812050ff] ? security_inode_permission+0x1f/0x30 [8116ea74] nameidata_to_filp+0x54/0x70 [811816b0] do_filp_open+0x700/0xd90 [810cfd42] ? audit_alloc_name+0x62/0x100 [8118fd20] ? mntput_no_expire+0x30/0x110 [8118dd12] ? alloc_fd+0x92/0x160 [8116e4b9] do_sys_open+0x69/0x140 [8116e5d0] sys_open+0x20/0x30 [8100b172] system_call_fastpath+0x16/0x1b Code: 89 74 24 18 0f 1f 44 00 00 48 c7 c3 40 5f 01 00 49 89 fc 49 89 f5 9c 58 0f 1f 44 00 00 48 89 c2 fa 66 0f 1f 44 00 00 49 89 55 00 49 8b 44 24 08 49 89 de 8b 40 18 4c 03 34 c5 40 46 b9 81 4c 89 RIP [8104fef2] task_rq_lock+0x42/0xa0 RSP 880017cbbb98 CR2: 0008 ---[ end trace a6f40a29150796da ]--- Amit Looking at 2.6.38+ without taking the time to audit my assumptions about the tty code, it would appear that we wait to register the driver until hvc_task is checked for IS_ERR(), and we only set hvc_driver after setting hvc_task. If it was some loosely ordered architecture I would worry about barriers. I also did a quick scan of patches from 2.6.32 to 38. I see several potential problems such as hvc_task is left IS_ERR, and we kill hvc_task before unregistering the tty, and we could end up with no kick with hvc_console backend registered (that should be ok, as there is still no tty driver to be called), but I don't see hvc_console in your module list and the only code to kill hvc_task and zero once its set is in the module
Re: hvc_console: Don't access hvc_task if not initialised
On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote: On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote: hvc_open() can be called without having any backing device. This results in a call to hvc_kick() which calls wake_up_process on a NULL pointer. How is hvc_open called without a hvc_driver registered to the tty layer? This gets reproduced in a couple of scenarios, I'm trying to get more information. Ensure hvc is initialised by checking for a non-NULL hvc_task before waking up the hvc thread. No if the task is missing the subsystem is really stuck. Put a check in open and refuse to open. This was found by an autotest run for virtio_console without having a console backend. stack trace please Yes, should have included that: BUG: unable to handle kernel NULL pointer dereference at 0008 IP: [8104fef2] task_rq_lock+0x42/0xa0 PGD 1d66a067 PUD 1d598067 PMD 0 Oops: [#1] SMP last sysfs file: /sys/devices/pci:00/:00:04.0/virtio1/virtio-ports/vport0p0/name CPU 0 Modules linked in: virtio_console virtio_net i2c_piix4 i2c_core ext4 mbcache jbd2 sr_mod cdrom virtio_blk pata_acpi ata_generic ata_piix virtio_pci virtio_ring virtio dm_mod [last unloaded: scsi_wait_scan] Modules linked in: virtio_console virtio_net i2c_piix4 i2c_core ext4 mbcache jbd2 sr_mod cdrom virtio_blk pata_acpi ata_generic ata_piix virtio_pci virtio_ring virtio dm_mod [last unloaded: scsi_wait_scan] Pid: 672, comm: console_check Not tainted 2.6.32-125.el6.x86_64 #1 KVM RIP: 0010:[8104fef2] [8104fef2] task_rq_lock+0x42/0xa0 RSP: 0018:880017cbbb98 EFLAGS: 00010082 RAX: 0282 RBX: 00015f40 RCX: 0002 RDX: 0282 RSI: 880017cbbbf0 RDI: RBP: 880017c8 R08: 88001bf39208 R09: 88001d67a000 R10: 0001 R11: R12: R13: 880017cbbbf0 R14: R15: 000f FS: 7f3592683700() GS:88000200() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 0008 CR3: 1d595000 CR4: 06f0 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400 Process console_check (pid: 672, threadinfo 880017cba000, task 88001f5fe100) Stack: 88001bf39000 0 880017cbbc28 8105c74c 88001d5966a0 8800 0 880017cbbbe8 813081c1 880017cbbc28 0282 Call Trace: [8105c74c] try_to_wake_up+0x3c/0x400 [813081c1] ? tty_ldisc_enable+0x31/0x40 [8105cb65] wake_up_process+0x15/0x20 [8131a10f] hvc_kick+0x1f/0x30 [8131a58a] hvc_open+0xca/0x120 [81302ba1] tty_open+0x211/0x510 [811754b5] chrdev_open+0x125/0x230 [81175390] ? chrdev_open+0x0/0x230 [8116e70a] __dentry_open+0x10a/0x360 [8120d022] ? selinux_inode_permission+0x72/0xb0 [812050ff] ? security_inode_permission+0x1f/0x30 [8116ea74] nameidata_to_filp+0x54/0x70 [811816b0] do_filp_open+0x700/0xd90 [810cfd42] ? audit_alloc_name+0x62/0x100 [8118fd20] ? mntput_no_expire+0x30/0x110 [8118dd12] ? alloc_fd+0x92/0x160 [8116e4b9] do_sys_open+0x69/0x140 [8116e5d0] sys_open+0x20/0x30 [8100b172] system_call_fastpath+0x16/0x1b Code: 89 74 24 18 0f 1f 44 00 00 48 c7 c3 40 5f 01 00 49 89 fc 49 89 f5 9c 58 0f 1f 44 00 00 48 89 c2 fa 66 0f 1f 44 00 00 49 89 55 00 49 8b 44 24 08 49 89 de 8b 40 18 4c 03 34 c5 40 46 b9 81 4c 89 RIP [8104fef2] task_rq_lock+0x42/0xa0 RSP 880017cbbb98 CR2: 0008 ---[ end trace a6f40a29150796da ]--- Amit ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: hvc_console: Don't access hvc_task if not initialised
[removed stable list from discussion]
On Thu, 24 Mar 2011 07:29:58 -, Amit Shah wrote:
hvc_open() can be called without having any backing device. This
results in a call to hvc_kick() which calls wake_up_process on a NULL
pointer.
How is hvc_open called without a hvc_driver registered to the tty layer?
Ensure hvc is initialised by checking for a non-NULL hvc_task
before waking up the hvc thread.
No if the task is missing the subsystem is really stuck. Put a check
in open and refuse to open.
This was found by an autotest run for virtio_console without having a
console backend.
stack trace please
CC: sta...@kernel.org
Signed-off-by: Amit Shah amit.s...@redhat.com
---
drivers/tty/hvc/hvc_console.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index e9cba13..b2cb5cc 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -286,6 +286,9 @@ EXPORT_SYMBOL_GPL(hvc_instantiate);
/* Wake the sleeping khvcd */
void hvc_kick(void)
{
+ if (!hvc_task)
+ return;
+
hvc_kicked = 1;
wake_up_process(hvc_task);
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
