From: Amit Shah <hidden> Date: 2011-03-24 07:30:22
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. Ensure hvc is initialised by checking for a non-NULL hvc_task
before waking up the hvc thread.
This was found by an autotest run for virtio_console without having a
console backend.
CC: stable@kernel.org
Signed-off-by: Amit Shah <redacted>
---
drivers/tty/hvc/hvc_console.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
@@ -286,6 +286,9 @@ EXPORT_SYMBOL_GPL(hvc_instantiate);/* Wake the sleeping khvcd */voidhvc_kick(void){+if(!hvc_task)+return;+hvc_kicked=1;wake_up_process(hvc_task);}
@@ -286,6 +286,9 @@ EXPORT_SYMBOL_GPL(hvc_instantiate);/* Wake the sleeping khvcd */voidhvc_kick(void){+if(!hvc_task)+return;+hvc_kicked=1;wake_up_process(hvc_task);}
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 exit.
milton
From: Amit Shah <hidden> Date: 2011-04-20 12:33:45
On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote:
On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote:
quoted
On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote:
quoted
On Thu, 24 Mar 2011 07:29:58 -0000, Amit Shah wrote:
quoted
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
On Wed, Apr 20, 2011 at 06:03:30PM +0530, Amit Shah wrote:
On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote:
quoted
On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote:
quoted
On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote:
quoted
On Thu, 24 Mar 2011 07:29:58 -0000, Amit Shah wrote:
quoted
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
From: Amit Shah <hidden> Date: 2011-04-20 17:06:24
On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote:
On Wed, Apr 20, 2011 at 06:03:30PM +0530, Amit Shah wrote:
quoted
On (Mon) 28 Mar 2011 [11:52:05], Milton Miller wrote:
quoted
On Fri, 25 Mar 2011 about 14:17:14 +0530, Amit Shah wrote:
quoted
On (Thu) 24 Mar 2011 [08:58:04], Milton Miller wrote:
quoted
On Thu, 24 Mar 2011 07:29:58 -0000, Amit Shah wrote:
quoted
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
From: Amit Shah <hidden> Date: 2011-04-27 06:31:59
On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote:
On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah [off-list ref] wrote:
quoted
On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote:
quoted
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
On Wed, Apr 27, 2011 at 12:01:47PM +0530, Amit Shah wrote:
On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote:
quoted
On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah [off-list ref] wrote:
quoted
On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote:
quoted
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
From: Amit Shah <hidden> Date: 2011-04-28 04:00:20
On (Wed) 27 Apr 2011 [17:09:34], Greg KH wrote:
On Wed, Apr 27, 2011 at 12:01:47PM +0530, Amit Shah wrote:
quoted
On (Wed) 27 Apr 2011 [14:31:29], Rusty Russell wrote:
quoted
On Wed, 20 Apr 2011 22:36:10 +0530, Amit Shah [off-list ref] wrote:
quoted
On (Wed) 20 Apr 2011 [07:34:35], Greg KH wrote:
quoted
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.