2001-12-21 22:51:54

by Jesper Juhl

[permalink] [raw]
Subject: [PATCH] console_loglevel broken on ia64 (and possibly other archs)

diff -ur linux-2.4.17-rc1-orig/arch/i386/mm/fault.c linux-2.4.17-rc1/arch/i386/mm/fault.c
--- linux-2.4.17-rc1-orig/arch/i386/mm/fault.c Wed Oct 10 00:13:03 2001
+++ linux-2.4.17-rc1/arch/i386/mm/fault.c Mon Dec 17 20:05:41 2001
@@ -27,8 +27,6 @@

extern void die(const char *,struct pt_regs *,long);

-extern int console_loglevel;
-
/*
* Ugly, ugly, but the goto's result in better assembly..
*/
diff -ur linux-2.4.17-rc1-orig/arch/parisc/kernel/traps.c linux-2.4.17-rc1/arch/parisc/kernel/traps.c
--- linux-2.4.17-rc1-orig/arch/parisc/kernel/traps.c Sun Sep 30 21:26:08 2001
+++ linux-2.4.17-rc1/arch/parisc/kernel/traps.c Mon Dec 17 20:05:41 2001
@@ -43,7 +43,6 @@

static inline void console_verbose(void)
{
- extern int console_loglevel;
console_loglevel = 15;
}

diff -ur linux-2.4.17-rc1-orig/include/linux/kernel.h linux-2.4.17-rc1/include/linux/kernel.h
--- linux-2.4.17-rc1-orig/include/linux/kernel.h Fri Dec 14 00:08:23 2001
+++ linux-2.4.17-rc1/include/linux/kernel.h Mon Dec 17 20:05:41 2001
@@ -36,6 +36,13 @@
#define KERN_INFO "<6>" /* informational */
#define KERN_DEBUG "<7>" /* debug-level messages */

+extern int console_printk[];
+
+#define console_loglevel (console_printk[0])
+#define default_message_loglevel (console_printk[1])
+#define minimum_console_loglevel (console_printk[2])
+#define default_console_loglevel (console_printk[3])
+
# define NORET_TYPE /**/
# define ATTRIB_NORET __attribute__((noreturn))
# define NORET_AND noreturn,
@@ -81,8 +88,6 @@

asmlinkage int printk(const char * fmt, ...)
__attribute__ ((format (printf, 1, 2)));
-
-extern int console_loglevel;

static inline void console_silent(void)
{
diff -ur linux-2.4.17-rc1-orig/kernel/printk.c linux-2.4.17-rc1/kernel/printk.c
--- linux-2.4.17-rc1-orig/kernel/printk.c Fri Dec 14 00:04:53 2001
+++ linux-2.4.17-rc1/kernel/printk.c Mon Dec 17 20:05:41 2001
@@ -16,6 +16,7 @@
* 01Mar01 Andrew Morton <[email protected]>
*/

+#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/tty.h>
#include <linux/tty_driver.h>
@@ -51,11 +52,12 @@

DECLARE_WAIT_QUEUE_HEAD(log_wait);

-/* Keep together for sysctl support */
-int console_loglevel = DEFAULT_CONSOLE_LOGLEVEL;
-int default_message_loglevel = DEFAULT_MESSAGE_LOGLEVEL;
-int minimum_console_loglevel = MINIMUM_CONSOLE_LOGLEVEL;
-int default_console_loglevel = DEFAULT_CONSOLE_LOGLEVEL;
+int console_printk[4] = {
+ DEFAULT_CONSOLE_LOGLEVEL, /* console_loglevel */
+ DEFAULT_MESSAGE_LOGLEVEL, /* default_message_loglevel */
+ MINIMUM_CONSOLE_LOGLEVEL, /* minimum_console_loglevel */
+ DEFAULT_CONSOLE_LOGLEVEL, /* default_console_loglevel */
+};

int oops_in_progress;


Attachments:
2.4.17-console_loglevel-1.patch (2.62 kB)

2001-12-25 09:51:06

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] console_loglevel broken on ia64 (and possibly other archs)

Hi!

> This patch fixes the console_loglevel variable(s) so that code that
> assumes the variables occupy continuous storage does not break (and
> overwrite other data).

It seems to me you are adding feature? And unneeded one, also.
Pavel

--
(about SSSCA) "I don't say this lightly. However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

2001-12-25 10:21:22

by Keith Owens

[permalink] [raw]
Subject: Re: [PATCH] console_loglevel broken on ia64 (and possibly other archs)

On Mon, 24 Dec 2001 23:35:15 +0100,
Pavel Machek <[email protected]> wrote:
>Jesper Juhl wrote
>
>> This patch fixes the console_loglevel variable(s) so that code that
>> assumes the variables occupy continuous storage does not break (and
>> overwrite other data).
>
>It seems to me you are adding feature? And unneeded one, also.

No, it is a bug fix. The existing code in kernel/printk.c has

/* Keep together for sysctl support */
int console_loglevel = DEFAULT_CONSOLE_LOGLEVEL;
int default_message_loglevel = DEFAULT_MESSAGE_LOGLEVEL;
int minimum_console_loglevel = MINIMUM_CONSOLE_LOGLEVEL;
int default_console_loglevel = DEFAULT_CONSOLE_LOGLEVEL;

and hopes that gcc will keep those variables together. The sysctl code
only addresses console_loglevel and assumes that the other three
variables are at the next addresses. There is no requirement for gcc
to keep unrelated variables in the order they are defined, on i386 it
does, on ia64 it does not. On ia64, doing
echo 1 2 3 4 > /proc/sys/kernel/printk
changes console_loglevel then corrupts three random words, whatever
variables gcc put after console_loglevel are destroyed.

2001-12-25 12:42:47

by kaih

[permalink] [raw]
Subject: Re: [PATCH] console_loglevel broken on ia64 (and possibly other archs)

[email protected] (Pavel Machek) wrote on 24.12.01 in <[email protected]>:

> > This patch fixes the console_loglevel variable(s) so that code that
> > assumes the variables occupy continuous storage does not break (and
> > overwrite other data).
>
> It seems to me you are adding feature? And unneeded one, also.

What feature would that be?


MfG Kai

2001-12-29 15:01:41

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] console_loglevel broken on ia64 (and possibly other archs)

On Mon, 2001-12-24 at 23:35, Pavel Machek wrote:
> > This patch fixes the console_loglevel variable(s) so that code that
> > assumes the variables occupy continuous storage does not break (and
> > overwrite other data).
>
> It seems to me you are adding feature? And unneeded one, also.
> Pavel

if you do

echo 6 4 1 7 > /proc/sys/kernel/printk

then you will overwrite console_loglevel and the next 3 ints. If the
next 3 ints are default_message_loglevel, minimum_console_loglevel &
default_console_loglevel then all is fine, but if these are not stored
in consecutive memory then you will corrupt other data instead - which
is a bug. By turning those into an array of ints then you guarantee that
the variables will occupy consecutive storage and thus the bug is no
more!
That is the purpose of the patch.

Keith Owens has confirmed this to be a problem on IA64 and that the
patch fixes the problem. I'm not aware of other architectures having
that problem, but with this patch it is impossible for them to have a
problem, and it has no ill effects as far as I can tell.

Thank you for your feedback!


--
Mvh. / Best regards
Jesper Juhl - [email protected]