[PATCH] register_console: prevent adding the same console twice

[PATCH] register_console: prevent adding the same console twice

[Andreas Bießmann]

This patch guards the console_drivers list to be corrupted. The
for_each_console() macro insist on a strictly forward list ended by NULL:

 con0->next->con1->next->NULL

Without this patch it may happen easily to destroy this list for example by
adding 'earlyprintk' twice. This will result in the following list:

 con0->next->con0

This in turn will result in an endless loop in console_unlock() later on by
printing the first __log_buf line endlessly.

Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kay Sievers <kay@vrfy.org>
Cc: stable@vger.kernel.org
---
 kernel/printk.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/printk.c b/kernel/printk.c
index 0b31715..f78bfcd 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -2254,6 +2254,14 @@ void register_console(struct console *newcon)
 	unsigned long flags;
 	struct console *bcon = NULL;
 
+	if (console_drivers)
+		for_each_console(bcon)
+			if (bcon == newcon) {
+				pr_warn("prevent adding console '%s%d' twice\n",
+					newcon->name, newcon->index);
+				return;
+			}
+
 	/*
 	 * before we register a new CON_BOOT console, make sure we don't
 	 * already have a valid console
-- 
1.7.10.4

Re: [PATCH] register_console: prevent adding the same console twice

[Ben Hutchings]

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

On Fri, 2013-03-22 at 11:10 +0100, Andreas Bießmann wrote:
> This patch guards the console_drivers list to be corrupted. The
> for_each_console() macro insist on a strictly forward list ended by NULL:
> 
>  con0->next->con1->next->NULL
> 
> Without this patch it may happen easily to destroy this list for example by
> adding 'earlyprintk' twice. This will result in the following list:
> 
>  con0->next->con0
> 
> This in turn will result in an endless loop in console_unlock() later on by
> printing the first __log_buf line endlessly.
> 
> Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: stable@vger.kernel.org
> ---
>  kernel/printk.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 0b31715..f78bfcd 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -2254,6 +2254,14 @@ void register_console(struct console *newcon)
>  	unsigned long flags;
>  	struct console *bcon = NULL;
>  
> +	if (console_drivers)
> +		for_each_console(bcon)
> +			if (bcon == newcon) {
> +				pr_warn("prevent adding console '%s%d' twice\n",
> +					newcon->name, newcon->index);

Since this is surely a bug in the calling driver, I think the warning
should be louder, i.e. use WARN.

Ben.

> +				return;
> +			}
> +
>  	/*
>  	 * before we register a new CON_BOOT console, make sure we don't
>  	 * already have a valid console

-- 
Ben Hutchings
Make three consecutive correct guesses and you will be considered an expert.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[PATCH v2] register_console: prevent adding the same console twice

[Andreas Bießmann]

This patch guards the console_drivers list to be corrupted. The
for_each_console() macro insist on a strictly forward list ended by NULL:

 con0->next->con1->next->NULL

Without this patch it may happen easily to destroy this list for example by
adding 'earlyprintk' twice, especially on embedded devices where the early
console is often a single static instance.  This will result in the following
list:

 con0->next->con0

This in turn will result in an endless loop in console_unlock() later on by
printing the first __log_buf line endlessly.

Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kay Sievers <kay@vrfy.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: stable@kernel.org
---
since v1:
 * use WARN() as Ben suggested
 * print 'already registered' instead of 'prevent registering'

 kernel/printk.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/printk.c b/kernel/printk.c
index abbdd9e..180ee25 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -2214,6 +2214,13 @@ void register_console(struct console *newcon)
 	unsigned long flags;
 	struct console *bcon = NULL;
 
+	if (console_drivers)
+		for_each_console(bcon)
+			if (WARN(bcon == newcon,
+					"console '%s%d' already registered\n",
+					bcon->name, bcon->index))
+				return;
+
 	/*
 	 * before we register a new CON_BOOT console, make sure we don't
 	 * already have a valid console
-- 
1.7.10.4

Re: [PATCH v2] register_console: prevent adding the same console twice

[Andreas Bießmann]

ping?

Am 2013-03-25 09:59, schrieb Andreas Bießmann:
> This patch guards the console_drivers list to be corrupted. The
> for_each_console() macro insist on a strictly forward list ended by 
> NULL:
> 
>  con0->next->con1->next->NULL
> 
> Without this patch it may happen easily to destroy this list for 
> example by
> adding 'earlyprintk' twice, especially on embedded devices where the 
> early
> console is often a single static instance.  This will result in the 
> following
> list:
> 
>  con0->next->con0
> 
> This in turn will result in an endless loop in console_unlock() later 
> on by
> printing the first __log_buf line endlessly.
> 
> Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: stable@kernel.org
> ---
> since v1:
>  * use WARN() as Ben suggested
>  * print 'already registered' instead of 'prevent registering'
> 
>  kernel/printk.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index abbdd9e..180ee25 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -2214,6 +2214,13 @@ void register_console(struct console *newcon)
>  	unsigned long flags;
>  	struct console *bcon = NULL;
> 
> +	if (console_drivers)
> +		for_each_console(bcon)
> +			if (WARN(bcon == newcon,
> +					"console '%s%d' already registered\n",
> +					bcon->name, bcon->index))
> +				return;
> +
>  	/*
>  	 * before we register a new CON_BOOT console, make sure we don't
>  	 * already have a valid console

[RESEND][PATCH v2] register_console: prevent adding the same console twice

[Andreas Bießmann]

This patch guards the console_drivers list to be corrupted. The
for_each_console() macro insist on a strictly forward list ended by NULL:

 con0->next->con1->next->NULL

Without this patch it may happen easily to destroy this list for example by
adding 'earlyprintk' twice, especially on embedded devices where the early
console is often a single static instance.  This will result in the following
list:

 con0->next->con0

This in turn will result in an endless loop in console_unlock() later on by
printing the first __log_buf line endlessly.

Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Kay Sievers <kay@vrfy.org>
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: stable@vger.kernel.org
---
since v1:
 * use WARN() as Ben suggested
 * print 'already registered' instead of 'prevent registering'

 kernel/printk/printk.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5b5a708..b4e8500 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2226,6 +2226,13 @@ void register_console(struct console *newcon)
 	struct console *bcon = NULL;
 	struct console_cmdline *c;
 
+	if (console_drivers)
+		for_each_console(bcon)
+			if (WARN(bcon == newcon,
+					"console '%s%d' already registered\n",
+					bcon->name, bcon->index))
+				return;
+
 	/*
 	 * before we register a new CON_BOOT console, make sure we don't
 	 * already have a valid console
-- 
1.7.10.4

Re: [RESEND][PATCH v2] register_console: prevent adding the same console twice

[Greg Kroah-Hartman]

On Fri, Aug 02, 2013 at 12:23:34PM +0200, Andreas Bießmann wrote:
> This patch guards the console_drivers list to be corrupted. The
> for_each_console() macro insist on a strictly forward list ended by NULL:
> 
>  con0->next->con1->next->NULL
> 
> Without this patch it may happen easily to destroy this list for example by
> adding 'earlyprintk' twice, especially on embedded devices where the early
> console is often a single static instance.  This will result in the following
> list:
> 
>  con0->next->con0
> 
> This in turn will result in an endless loop in console_unlock() later on by
> printing the first __log_buf line endlessly.
> 
> Signed-off-by: Andreas Bießmann <andreas@biessmann.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: stable@vger.kernel.org

It's a nice "feature", but I fail to see how this is worthy of going
into the stable tree, as it's not fixing a kernel error, only a typo by
a user.

thanks,

greg k-h

Re: [RESEND][PATCH v2] register_console: prevent adding the same console twice

[Greg Kroah-Hartman]

On Fri, Aug 02, 2013 at 12:23:34PM +0200, Andreas Bie�mann wrote:
> This patch guards the console_drivers list to be corrupted. The
> for_each_console() macro insist on a strictly forward list ended by NULL:
> 
>  con0->next->con1->next->NULL
> 
> Without this patch it may happen easily to destroy this list for example by
> adding 'earlyprintk' twice, especially on embedded devices where the early
> console is often a single static instance.  This will result in the following
> list:
> 
>  con0->next->con0
> 
> This in turn will result in an endless loop in console_unlock() later on by
> printing the first __log_buf line endlessly.
> 
> Signed-off-by: Andreas Bie�mann <andreas@biessmann.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Kay Sievers <kay@vrfy.org>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: stable@vger.kernel.org

It's a nice "feature", but I fail to see how this is worthy of going
into the stable tree, as it's not fixing a kernel error, only a typo by
a user.

thanks,

greg k-h