[opensuse-kernel] TTY_PRINTK
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own). Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module. Otherwise we'll have to build it into kernel always although its usage must be pretty rare. Takashi -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Takashi, Andreas, Le Tuesday 01 April 2014 à 15:19 +0200, Takashi Iwai a écrit :
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module.
No good reason that I can see. I tried turning TTY_PRINTK into a tristate option and building it as a module, and did not get any error. I didn't actually test it though, and won't have time for that. Should I push that experimental patch to our master kernel branch for someone else to test it? Or I can just share the patch if someone wants to test it first.
Otherwise we'll have to build it into kernel always although its usage must be pretty rare.
It's fairly small so not necessarily a big deal, unless there are security concerns. Anyway we could always enable it in our debug kernel flavors only. -- Jean Delvare SUSE L3 Support -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
At Wed, 02 Apr 2014 10:01:43 +0200, Jean Delvare wrote:
Hi Takashi, Andreas,
Le Tuesday 01 April 2014 à 15:19 +0200, Takashi Iwai a écrit :
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module.
No good reason that I can see. I tried turning TTY_PRINTK into a tristate option and building it as a module, and did not get any error. I didn't actually test it though, and won't have time for that.
Ditto, I did make a quick fix patch and load/unload without real testing, too :) FWIW, my patch is attached below.
Should I push that experimental patch to our master kernel branch for someone else to test it? Or I can just share the patch if someone wants to test it first.
I guess it's better to submit to upstream at first. I see that Andreas' question came from the LKML thread: https://lkml.org/lkml/2014/3/29/34
Otherwise we'll have to build it into kernel always although its usage must be pretty rare.
It's fairly small so not necessarily a big deal, unless there are security concerns.
Right, but it'd be better if this could be integrated as a module from the beginning.
Anyway we could always enable it in our debug kernel flavors only.
True.
thanks,
Takashi
---
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 1386749b48ff..97816b133c7f 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -40,7 +40,7 @@ config SGI_MBCS
source "drivers/tty/serial/Kconfig"
config TTY_PRINTK
- bool "TTY driver to output user messages via printk"
+ tristate "TTY driver to output user messages via printk"
depends on EXPERT && TTY
default n
---help---
diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
index daea84c41743..4ae77317a937 100644
--- a/drivers/char/ttyprintk.c
+++ b/drivers/char/ttyprintk.c
@@ -17,7 +17,7 @@
#include
Hi Takashi, Le Wednesday 02 April 2014 à 10:07 +0200, Takashi Iwai a écrit :
At Wed, 02 Apr 2014 10:01:43 +0200, Jean Delvare wrote:
Hi Takashi, Andreas,
Le Tuesday 01 April 2014 à 15:19 +0200, Takashi Iwai a écrit :
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module.
No good reason that I can see. I tried turning TTY_PRINTK into a tristate option and building it as a module, and did not get any error. I didn't actually test it though, and won't have time for that.
Ditto, I did make a quick fix patch and load/unload without real testing, too :) FWIW, my patch is attached below.
Ah, you went further than I did, so let's continue with your work.
Should I push that experimental patch to our master kernel branch for someone else to test it? Or I can just share the patch if someone wants to test it first.
I guess it's better to submit to upstream at first.
Agreed.
Otherwise we'll have to build it into kernel always although its usage must be pretty rare.
It's fairly small so not necessarily a big deal, unless there are security concerns.
Right, but it'd be better if this could be integrated as a module from the beginning.
Certainly :-)
--- diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 1386749b48ff..97816b133c7f 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -40,7 +40,7 @@ config SGI_MBCS source "drivers/tty/serial/Kconfig"
config TTY_PRINTK - bool "TTY driver to output user messages via printk" + tristate "TTY driver to output user messages via printk" depends on EXPERT && TTY default n ---help--- diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c index daea84c41743..4ae77317a937 100644 --- a/drivers/char/ttyprintk.c +++ b/drivers/char/ttyprintk.c @@ -17,7 +17,7 @@ #include
#include #include -#include +#include struct ttyprintk_port { struct tty_port port; @@ -175,6 +175,14 @@ static struct tty_port_operations null_ops = { };
static struct tty_driver *ttyprintk_driver;
+static void ttyprintk_exit(void) +{ + tty_unregister_driver(ttyprintk_driver); + put_tty_driver(ttyprintk_driver); + tty_port_destroy(&tpk_port.port); + ttyprintk_driver = NULL; +} + static int __init ttyprintk_init(void) { int ret = -ENOMEM; @@ -210,10 +218,11 @@ static int __init ttyprintk_init(void) return 0;
error: - tty_unregister_driver(ttyprintk_driver); - put_tty_driver(ttyprintk_driver); - tty_port_destroy(&tpk_port.port); - ttyprintk_driver = NULL; + ttyprintk_exit(); return ret; }
I'm afraid you'll need a preparatory patch to fix the code first. The error path is wrong, tty_unregister_driver() shouldn't be called there. Not sure about put_tty_driver(). ttyprintk_driver = NULL is useless.
+ device_initcall(ttyprintk_init); +module_exit(ttyprintk_exit); + +MODULE_LICENSE("GPL");
MODULE_AUTHOR() would be good to have too? -- Jean Delvare SUSE L3 Support -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
At Wed, 02 Apr 2014 10:20:25 +0200, Jean Delvare wrote:
Hi Takashi,
Le Wednesday 02 April 2014 à 10:07 +0200, Takashi Iwai a écrit :
At Wed, 02 Apr 2014 10:01:43 +0200, Jean Delvare wrote:
Hi Takashi, Andreas,
Le Tuesday 01 April 2014 à 15:19 +0200, Takashi Iwai a écrit :
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module.
No good reason that I can see. I tried turning TTY_PRINTK into a tristate option and building it as a module, and did not get any error. I didn't actually test it though, and won't have time for that.
Ditto, I did make a quick fix patch and load/unload without real testing, too :) FWIW, my patch is attached below.
Ah, you went further than I did, so let's continue with your work.
Should I push that experimental patch to our master kernel branch for someone else to test it? Or I can just share the patch if someone wants to test it first.
I guess it's better to submit to upstream at first.
Agreed.
Otherwise we'll have to build it into kernel always although its usage must be pretty rare.
It's fairly small so not necessarily a big deal, unless there are security concerns.
Right, but it'd be better if this could be integrated as a module from the beginning.
Certainly :-)
--- diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 1386749b48ff..97816b133c7f 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -40,7 +40,7 @@ config SGI_MBCS source "drivers/tty/serial/Kconfig"
config TTY_PRINTK - bool "TTY driver to output user messages via printk" + tristate "TTY driver to output user messages via printk" depends on EXPERT && TTY default n ---help--- diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c index daea84c41743..4ae77317a937 100644 --- a/drivers/char/ttyprintk.c +++ b/drivers/char/ttyprintk.c @@ -17,7 +17,7 @@ #include
#include #include -#include +#include struct ttyprintk_port { struct tty_port port; @@ -175,6 +175,14 @@ static struct tty_port_operations null_ops = { };
static struct tty_driver *ttyprintk_driver;
+static void ttyprintk_exit(void) +{ + tty_unregister_driver(ttyprintk_driver); + put_tty_driver(ttyprintk_driver); + tty_port_destroy(&tpk_port.port); + ttyprintk_driver = NULL; +} + static int __init ttyprintk_init(void) { int ret = -ENOMEM; @@ -210,10 +218,11 @@ static int __init ttyprintk_init(void) return 0;
error: - tty_unregister_driver(ttyprintk_driver); - put_tty_driver(ttyprintk_driver); - tty_port_destroy(&tpk_port.port); - ttyprintk_driver = NULL; + ttyprintk_exit(); return ret; }
I'm afraid you'll need a preparatory patch to fix the code first. The error path is wrong, tty_unregister_driver() shouldn't be called there. Not sure about put_tty_driver(). ttyprintk_driver = NULL is useless.
Oh right. put_tty_driver() is still needed for releasing an object allocated by tty_alloc_driver(). OK, I'm going to fix it first.
+ device_initcall(ttyprintk_init); +module_exit(ttyprintk_exit); + +MODULE_LICENSE("GPL");
MODULE_AUTHOR() would be good to have too?
Depends. I tend to skip this for my own code nowadays. thanks, Takashi -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Le Wednesday 02 April 2014 à 10:01 +0200, Jean Delvare a écrit :
Hi Takashi, Andreas,
Le Tuesday 01 April 2014 à 15:19 +0200, Takashi Iwai a écrit :
At Mon, 31 Mar 2014 12:33:46 +0200, Andreas Schwab wrote:
Would it make sense to enable TTY_PRINTK? It can be used to redirect the system console to the same channel printk uses (write-only, of course), eg. it can be used to capture user-level console messages via netconsole (which doesn't provide a tty device on its own).
I wonder why this can't be a module.
Otherwise we'll have to build it into kernel always although its usage must be pretty rare.
It's fairly small so not necessarily a big deal, unless there are security concerns.
Anyway we could always enable it in our debug kernel flavors only.
I've done that for now. Once TTY_PRINTK can be built as a module, we can enable it in other flavors as well. Takashi, I'll let you handle that, thanks. -- Jean Delvare SUSE L3 Support -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
participants (3)
-
Andreas Schwab -
Jean Delvare -
Takashi Iwai