linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port()
@ 2014-02-28 13:21 Geert Uytterhoeven
  2014-02-28 13:21 ` [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path Geert Uytterhoeven
  2014-03-11  1:06 ` [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Peter Hurley
  0 siblings, 2 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-02-28 13:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Laurent Pinchart, linux-serial, linux-sh, linux-kernel,
	Geert Uytterhoeven

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

If the serial port being removed is used as a console, it must also be
unregistered from the console subsystem using unregister_console().

uart_ops.release_port() will release resources (e.g. iounmap() the serial
port registers), causing a crash on subsequent kernel output if the console
is still registered.

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
 drivers/tty/serial/serial_core.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index ece2049bd270..8ece7f14d89d 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2677,6 +2677,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 		tty_vhangup(port->tty);
 
 	/*
+	 * If the port is used as a console, unregister it
+	 */
+	if (uart_console(uport))
+		unregister_console(uport->cons);
+
+	/*
 	 * Free the port IO and memory resources, if any.
 	 */
 	if (uport->type != PORT_UNKNOWN)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path
  2014-02-28 13:21 [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Geert Uytterhoeven
@ 2014-02-28 13:21 ` Geert Uytterhoeven
  2014-03-06  4:19   ` Simon Horman
  2014-03-11  1:06 ` [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Peter Hurley
  1 sibling, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-02-28 13:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Laurent Pinchart, linux-serial, linux-sh, linux-kernel,
	Geert Uytterhoeven

From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

If cpufreq_register_notifier() fails, we have to remove the port added by
sci_probe_single(), which is not done by sci_cleanup_single().

Else the serial port stays active from the point of view of the serial
subsystem, and it may crash when userspace getty is started, or when the
loadable driver module is unloaded.

This was introduced by commit 6dae14216c85eea13db7b12c469475c5d30e5499
("serial: sh-sci: Fix probe error paths").

Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
---
This depends on "serial_core: Unregister console in uart_remove_one_port()"
when using a serial console.
---
 drivers/tty/serial/sh-sci.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index be33d2b0613b..7958115e6a51 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2564,6 +2564,7 @@ static int sci_probe(struct platform_device *dev)
 	ret = cpufreq_register_notifier(&sp->freq_transition,
 					CPUFREQ_TRANSITION_NOTIFIER);
 	if (unlikely(ret < 0)) {
+		uart_remove_one_port(&sci_uart_driver, &sp->port);
 		sci_cleanup_single(sp);
 		return ret;
 	}
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path
  2014-02-28 13:21 ` [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path Geert Uytterhoeven
@ 2014-03-06  4:19   ` Simon Horman
  2014-03-06  4:34     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2014-03-06  4:19 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Kroah-Hartman, Jiri Slaby, Laurent Pinchart, linux-serial,
	linux-sh, linux-kernel, Geert Uytterhoeven

On Fri, Feb 28, 2014 at 02:21:33PM +0100, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> If cpufreq_register_notifier() fails, we have to remove the port added by
> sci_probe_single(), which is not done by sci_cleanup_single().
> 
> Else the serial port stays active from the point of view of the serial
> subsystem, and it may crash when userspace getty is started, or when the
> loadable driver module is unloaded.
> 
> This was introduced by commit 6dae14216c85eea13db7b12c469475c5d30e5499
> ("serial: sh-sci: Fix probe error paths").
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>

Acked-by: Simon Horman <horms+renesas@verge.net.au>

This looks reasonable to me.

Greg, could you take this one?


> ---
> This depends on "serial_core: Unregister console in uart_remove_one_port()"
> when using a serial console.
> ---
>  drivers/tty/serial/sh-sci.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index be33d2b0613b..7958115e6a51 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -2564,6 +2564,7 @@ static int sci_probe(struct platform_device *dev)
>  	ret = cpufreq_register_notifier(&sp->freq_transition,
>  					CPUFREQ_TRANSITION_NOTIFIER);
>  	if (unlikely(ret < 0)) {
> +		uart_remove_one_port(&sci_uart_driver, &sp->port);
>  		sci_cleanup_single(sp);
>  		return ret;
>  	}
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path
  2014-03-06  4:19   ` Simon Horman
@ 2014-03-06  4:34     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2014-03-06  4:34 UTC (permalink / raw)
  To: Simon Horman
  Cc: Geert Uytterhoeven, Jiri Slaby, Laurent Pinchart, linux-serial,
	linux-sh, linux-kernel, Geert Uytterhoeven

On Thu, Mar 06, 2014 at 01:19:13PM +0900, Simon Horman wrote:
> On Fri, Feb 28, 2014 at 02:21:33PM +0100, Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> > 
> > If cpufreq_register_notifier() fails, we have to remove the port added by
> > sci_probe_single(), which is not done by sci_cleanup_single().
> > 
> > Else the serial port stays active from the point of view of the serial
> > subsystem, and it may crash when userspace getty is started, or when the
> > loadable driver module is unloaded.
> > 
> > This was introduced by commit 6dae14216c85eea13db7b12c469475c5d30e5499
> > ("serial: sh-sci: Fix probe error paths").
> > 
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> 
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> This looks reasonable to me.
> 
> Greg, could you take this one?

Will do, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port()
  2014-02-28 13:21 [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Geert Uytterhoeven
  2014-02-28 13:21 ` [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path Geert Uytterhoeven
@ 2014-03-11  1:06 ` Peter Hurley
  2014-03-11  3:43   ` Peter Hurley
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Hurley @ 2014-03-11  1:06 UTC (permalink / raw)
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
  Cc: Laurent Pinchart, linux-serial, linux-sh, linux-kernel,
	Geert Uytterhoeven, One Thousand Gnomes

[ +cc Alan ]

On 02/28/2014 08:21 AM, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>
> If the serial port being removed is used as a console, it must also be
> unregistered from the console subsystem using unregister_console().
>
> uart_ops.release_port() will release resources (e.g. iounmap() the serial
> port registers), causing a crash on subsequent kernel output if the console
> is still registered.

This brings up an interesting point:  where are the serial_core drivers
indicating to the tty core that the port is bound to a console?
IOW, I cannot find a single 'port->console = 1' anywhere in the serial tree.

It would be bad if the uart port disappears while a console is running
on it...

> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
> ---
>   drivers/tty/serial/serial_core.c |    6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index ece2049bd270..8ece7f14d89d 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2677,6 +2677,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>   		tty_vhangup(port->tty);
>
>   	/*
> +	 * If the port is used as a console, unregister it
> +	 */
> +	if (uart_console(uport))
> +		unregister_console(uport->cons);
> +
> +	/*
>   	 * Free the port IO and memory resources, if any.
>   	 */
>   	if (uport->type != PORT_UNKNOWN)
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port()
  2014-03-11  1:06 ` [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Peter Hurley
@ 2014-03-11  3:43   ` Peter Hurley
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Hurley @ 2014-03-11  3:43 UTC (permalink / raw)
  To: Geert Uytterhoeven, Greg Kroah-Hartman, Jiri Slaby
  Cc: Laurent Pinchart, linux-serial, linux-sh, linux-kernel,
	Geert Uytterhoeven, One Thousand Gnomes

On 03/10/2014 09:06 PM, Peter Hurley wrote:
> [ +cc Alan ]
>
> On 02/28/2014 08:21 AM, Geert Uytterhoeven wrote:
>> From: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>>
>> If the serial port being removed is used as a console, it must also be
>> unregistered from the console subsystem using unregister_console().
>>
>> uart_ops.release_port() will release resources (e.g. iounmap() the serial
>> port registers), causing a crash on subsequent kernel output if the console
>> is still registered.
>
> This brings up an interesting point:  where are the serial_core drivers
> indicating to the tty core that the port is bound to a console?
> IOW, I cannot find a single 'port->console = 1' anywhere in the serial tree.

Answering half my own question: I see that serial_core doesn't use
tty_port_shutdown() but rather has it's own uart_shutdown() equivalent, and
thus doesn't use 'port->console'.

Ok.

But I still don't see what prevents a uart_port_shutdown() on a running
serial console.

Regards,
Peter Hurley

> It would be bad if the uart port disappears while a console is running
> on it...
>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@linux-m68k.org>
>> ---
>>   drivers/tty/serial/serial_core.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index ece2049bd270..8ece7f14d89d 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -2677,6 +2677,12 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
>>           tty_vhangup(port->tty);
>>
>>       /*
>> +     * If the port is used as a console, unregister it
>> +     */
>> +    if (uart_console(uport))
>> +        unregister_console(uport->cons);
>> +
>> +    /*
>>        * Free the port IO and memory resources, if any.
>>        */
>>       if (uport->type != PORT_UNKNOWN)
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-03-11  3:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 13:21 [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Geert Uytterhoeven
2014-02-28 13:21 ` [PATCH 2/2] serial: sh-sci: Add missing call to uart_remove_one_port() in failure path Geert Uytterhoeven
2014-03-06  4:19   ` Simon Horman
2014-03-06  4:34     ` Greg Kroah-Hartman
2014-03-11  1:06 ` [PATCH 1/2] serial_core: Unregister console in uart_remove_one_port() Peter Hurley
2014-03-11  3:43   ` Peter Hurley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).