Re: linux-2.4.0-test11 and sysklogd-1.3-31

From: Georg Nikodym (georgn@somanetworks.com)
Date: Mon Dec 11 2000 - 20:13:46 EST


>>>>> "KO" == Keith Owens <kaos@ocs.com.au> writes:

 KO> klogd maps the kernel messages from <n>text to syslog levels and
 KO> does some fiddling with kernel log levels at start up. It needs
 KO> to be more than a simple 'cat'. When symbol handling was added
 KO> to klogd, ksymoops was built into the kernel and very unreliable.
 KO> Since then ksymoops has been moved to a separate package and is
 KO> now reliable. Alas oops handling in sysklogd has not kept up to
 KO> date and is now the problem area.

Thanks for the background.

 KO> Linus, please do not apply.

 KO> User space applications _must_ not include kernel headers. Even
 KO> modutils does not include linux/module.h, it has its own portable
 KO> (kernels 2.0 - 2.4) version.

 KO> I have strongly recommended to the sysklogd maintainers that they
 KO> strip all the symbol handling from klogd. The oops decoding in
 KO> klogd is hopelessly out of date and broken.

Here's a patch (against sysklogd-1.3-31) that completely tear out the
symbol processing code.

Additionally, after application, the following files may be removed:

        ksyms.h
        ksym.c
        ksym_mod.c

diff -Nru a/src/sysklogd-1.3-31/Makefile b/src/sysklogd-1.3-31/Makefile
--- a/src/sysklogd-1.3-31/Makefile Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/Makefile Mon Dec 11 19:32:22 2000
@@ -63,9 +63,8 @@
 syslogd: syslogd.o pidfile.o
         ${CC} ${LDFLAGS} -o syslogd syslogd.o pidfile.o ${LIBS}
 
-klogd: klogd.o syslog.o pidfile.o ksym.o ksym_mod.o
- ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o \
- ksym_mod.o ${LIBS}
+klogd: klogd.o syslog.o pidfile.o ksym.o
+ ${CC} ${LDFLAGS} -o klogd klogd.o syslog.o pidfile.o ksym.o ${LIBS}
 
 syslog_tst: syslog_tst.o
         ${CC} ${LDFLAGS} -o syslog_tst syslog_tst.o
diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:22 2000
@@ -415,7 +415,6 @@
         if (symbol_lookup) {
                 if ( reload_symbols > 1 )
                         InitKsyms(symfile);
- InitMsyms();
         }
         reload_symbols = change_state = 0;
         return;
@@ -1059,7 +1058,6 @@
         {
                 if (symbol_lookup) {
                         InitKsyms(symfile);
- InitMsyms();
                 }
                 if ( (logsrc = GetKernelLogSrc()) == kernel )
                         LogKernelLine();
@@ -1075,7 +1073,6 @@
         logsrc = GetKernelLogSrc();
         if (symbol_lookup) {
                 InitKsyms(symfile);
- InitMsyms();
         }
 
         /* The main loop. */
diff -Nru a/src/sysklogd-1.3-31/ksym.c b/src/sysklogd-1.3-31/ksym.c
--- a/src/sysklogd-1.3-31/ksym.c Mon Dec 11 19:32:22 2000
+++ b/src/sysklogd-1.3-31/ksym.c Mon Dec 11 19:32:22 2000
@@ -656,9 +656,6 @@
                 last = sym_array[lp].name;
         }
 
- if ( (last = LookupModuleSymbol(value, sym)) != (char *) 0 )
- return(last);
-
         return((char *) 0);
 }
 
@@ -749,7 +746,7 @@
          * open for patches.
          */
         if ( i_am_paranoid &&
- (strstr(line, "Oops:") != (char *) 0) && !InitMsyms() )
+ (strstr(line, "Oops:") != (char *) 0) )
                 Syslog(LOG_WARNING, "Cannot load kernel module symbols.\n");
         
 
diff -Nru a/src/sysklogd-1.3-31/ksyms.h b/src/sysklogd-1.3-31/ksyms.h
--- a/src/sysklogd-1.3-31/ksyms.h Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/ksyms.h Mon Dec 11 19:32:23 2000
@@ -32,4 +32,3 @@
 
 /* Function prototypes. */
 extern char * LookupSymbol(unsigned long, struct symbol *);
-extern char * LookupModuleSymbol(unsigned long int, struct symbol *);
diff -Nru a/src/sysklogd-1.3-31/klogd.8 b/src/sysklogd-1.3-31/klogd.8
--- a/src/sysklogd-1.3-31/klogd.8 Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.8 Mon Dec 11 19:32:23 2000
@@ -3,6 +3,8 @@
 .\" Sun Jul 30 01:35:55 MET: Martin Schulze: Updates
 .\" Sun Nov 19 23:22:21 MET: Martin Schulze: Updates
 .\" Mon Aug 19 09:42:08 CDT 1996: Dr. G.W. Wettstein: Updates
+.\" Mon Dec 11 2000: Georg Nikodym: Removal of documentation related to
+.\" symbol resolution (the code has been removed).
 .\"
 .TH KLOGD 8 "24 November 1995" "Version 1.3" "Linux System Administration"
 .SH NAME
@@ -17,16 +19,10 @@
 .RB [ " \-f "
 .I fname
 ]
-.RB [ " \-iI " ]
 .RB [ " \-n " ]
 .RB [ " \-o " ]
-.RB [ " \-p " ]
 .RB [ " \-s " ]
-.RB [ " \-k "
-.I fname
-]
 .RB [ " \-v " ]
-.RB [ " \-x " ]
 .LP
 .SH DESCRIPTION
 .B klogd
@@ -45,12 +41,6 @@
 .BI "\-f " file
 Log messages to the specified filename rather than to the syslog facility.
 .TP
-.BI "\-i \-I"
-Signal the currently executing klogd daemon. Both of these switches control
-the loading/reloading of symbol information. The \-i switch signals the
-daemon to reload the kernel module symbols. The \-I switch signals for a
-reload of both the static kernel symbols and the kernel module symbols.
-.TP
 .B "\-n"
 Avoid auto-backgrounding. This is needed especially if the
 .B klogd
@@ -62,24 +52,12 @@
 all the messages that are found in the kernel message buffers. After
 a single read and log cycle the daemon exits.
 .TP
-.B "-p"
-Enable paranoia. This option controls when klogd loads kernel module symbol
-information. Setting this switch causes klogd to load the kernel module
-symbol information whenever an Oops string is detected in the kernel message
-stream.
-.TP
 .B "-s"
 Force \fBklogd\fP to use the system call interface to the kernel message
 buffers.
 .TP
-.BI "\-k " file
-Use the specified file as the source of kernel symbol information.
-.TP
 .B "\-v"
 Print version and exit.
-.TP
-.B "\-x"
-Omits EIP translation and there doesn't read the System.map.
 .LP
 .SH OVERVIEW
 The functionality of klogd has been typically incorporated into other
@@ -214,37 +192,6 @@
 .B ksymoops
 program which is included in the kernel sources.
 
-As a convenience
-.B klogd
-will attempt to resolve kernel numeric addresses to their symbolic
-forms if a kernel symbol table is available at execution time. A
-symbol table may be specified by using the \fB\-k\fR switch on the
-command line. If a symbol file is not explicitly specified the
-following filenames will be tried:
-
-.nf
-.I /boot/System.map
-.I /System.map
-.I /usr/src/linux/System.map
-.fi
-
-Version information is supplied in the system maps as of kernel
-1.3.43. This version information is used to direct an intelligent
-search of the list of symbol tables. This feature is useful since it
-provides support for both production and experimental kernels.
-
-For example a production kernel may have its map file stored in
-/boot/System.map. If an experimental or test kernel is compiled with
-the sources in the 'standard' location of /usr/src/linux the system
-map will be found in /usr/src/linux/System.map. When klogd starts
-under the experimental kernel the map in /boot/System.map will be
-bypassed in favor of the map in /usr/src/linux/System.map.
-
-Modern kernels as of 1.3.43 properly format important kernel addresses
-so that they will be recognized and translated by klogd. Earlier
-kernels require a source code patch be applied to the kernel sources.
-This patch is supplied with the sysklogd sources.
-
 The process of analyzing kernel protections faults works very well
 with a static kernel. Additional difficulties are encountered when
 attempting to diagnose errors which occur in loadable kernel modules.
@@ -262,70 +209,6 @@
 loadable module. Without this location map it is not possible for a
 kernel developer to determine what went wrong if a protection fault
 involves a kernel module.
-
-.B klogd
-has support for dealing with the problem of diagnosing protection
-faults in kernel loadable modules. At program start time or in
-response to a signal the daemon will interrogate the kernel for a
-listing of all modules loaded and the addresses in memory they are
-loaded at. Individual modules can also register the locations of
-important functions when the module is loaded. The addresses of these
-exported symbols are also determined during this interrogation
-process.
-
-When a protection fault occurs an attempt will be made to resolve
-kernel addresses from the static symbol table. If this fails the
-symbols from the currently loaded modules are examined in an attempt
-to resolve the addresses. At the very minimum this allows klogd to
-indicate which loadable module was responsible for generating the
-protection fault. Additional information may be available if the
-module developer chose to export symbol information from the module.
-
-Proper and accurate resolution of addresses in kernel modules requires
-that
-.B klogd
-be informed whenever the kernel module status changes. The
-.B \-i
-and
-.B \-I
-switches can be used to signal the currently executing daemon that
-symbol information be reloaded. Of most importance to proper
-resolution of module symbols is the
-.B \-i
-switch. Each time a kernel module is loaded or removed from the
-kernel the following command should be executed:
-
-.nf
-.I klogd \-i
-.fi
-
-The
-.B \-p
-switch can also be used to insure that module symbol information is up
-to date. This switch instructs
-.B klogd
-to reload the module symbol information whenever a protection fault
-is detected. Caution should be used before invoking the program in
-\'paranoid\' mode. The stability of the kernel and the operating
-environment is always under question when a protection fault occurs.
-Since the klogd daemon must execute system calls in order to read the
-module symbol information there is the possibility that the system may
-be too unstable to capture useful information. A much better policy
-is to insure that klogd is updated whenever a module is loaded or
-unloaded. Having uptodate symbol information loaded increases the
-probability of properly resolving a protection fault if it should occur.
-
-Included in the sysklogd source distribution is a patch to the
-modules-2.0.0 package which allows the
-.B insmod,
-.B rmmod
-and
-.B modprobe
-utilities to automatically signal
-.B klogd
-whenever a module is inserted or removed from the kernel. Using this
-patch will insure that the symbol information maintained in klogd is
-always consistent with the current kernel state.
 .PP
 .SH SIGNAL HANDLING
 The
@@ -363,26 +246,6 @@
 .B LOG_INFO
 priority
 documenting the start/stop of logging.
-
-The
-.BR SIGUSR1 " and " SIGUSR2
-signals are used to initiate loading/reloading of kernel symbol information.
-Receipt of the
-.B SIGUSR1
-signal will cause the kernel module symbols to be reloaded. Signaling the
-daemon with
-.B SIGUSR2
-will cause both the static kernel symbols and the kernel module symbols to
-be reloaded.
-
-Provided that the System.map file is placed in an appropriate location the
-signal of generally greatest usefulness is the
-.B SIGUSR1
-signal. This signal is designed to be used to signal the daemon when kernel
-modules are loaded/unloaded. Sending this signal to the daemon after a
-kernel module state change will insure that proper resolution of symbols will
-occur if a protection fault occurs in the address space occupied by a kernel
-module.
 .LP
 .SH FILES
 .PD 0
diff -Nru a/src/sysklogd-1.3-31/klogd.c b/src/sysklogd-1.3-31/klogd.c
--- a/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.c Mon Dec 11 19:32:23 2000
@@ -213,6 +213,10 @@
  * Shortened LOG_LINE_LENGTH in order to get long lines splitted
  * up earlier and syslogd has a better chance concatenating them
  * together again.
+ *
+ * Mon Dec 11, 2000: Georg Nikodym <georgn@somanetworks.com>
+ * Removed all support for symbol resolution at the suggestion of
+ * Keith Owens.
  */
 
 
@@ -229,7 +233,6 @@
 #include <paths.h>
 #include <stdlib.h>
 #include "klogd.h"
-#include "ksyms.h"
 #ifndef TESTING
 #include "pidfile.h"
 #endif
@@ -260,16 +263,13 @@
                 change_state = 0,
                 terminate = 0,
                 caught_TSTP = 0,
- reload_symbols = 0,
                 console_log_level = 6;
 
 static int use_syscall = 0,
                 one_shot = 0,
- symbol_lookup = 1,
                 no_fork = 0; /* don't fork - don't run in daemon mode */
 
-static char *symfile = (char *) 0,
- log_buffer[LOG_BUFFER_SIZE];
+static char log_buffer[LOG_BUFFER_SIZE];
 
 static FILE *output_file = (FILE *) 0;
 
@@ -287,7 +287,6 @@
 extern void reload_daemon(int sig);
 static void Terminate(void);
 static void SignalDaemon(int);
-static void ReloadSymbols(void);
 static void ChangeLogging(void);
 static enum LOGSRC GetKernelLogSrc(void);
 static void LogLine(char *ptr, int len);
@@ -363,12 +362,10 @@
 
 {
         change_state = 1;
- reload_symbols = 1;
 
 
         if ( sig == SIGUSR2 )
         {
- ++reload_symbols;
                 signal(SIGUSR2, reload_daemon);
         }
         else
@@ -409,17 +406,6 @@
 }
 
 
-static void ReloadSymbols()
-
-{
- if (symbol_lookup) {
- if ( reload_symbols > 1 )
- InitKsyms(symfile);
- }
- reload_symbols = change_state = 0;
- return;
-}
-
 
 static void ChangeLogging(void)
 
@@ -432,13 +418,6 @@
         Syslog(LOG_INFO, "klogd %s-%s, ---------- state change ----------\n", \
                VERSION, PATCHLEVEL);
 
- /* Reload symbols. */
- if ( reload_symbols > 0 )
- {
- ReloadSymbols();
- return;
- }
-
         /* Stop kernel logging. */
         if ( caught_TSTP == 1 )
         {
@@ -627,24 +606,11 @@
 /*
  * Messages are separated by "\n". Messages longer than
  * LOG_LINE_LENGTH are broken up.
- *
- * Kernel symbols show up in the input buffer as : "[<aaaaaa>]",
- * where "aaaaaa" is the address. These are replaced with
- * "[symbolname+offset/size]" in the output line - symbolname,
- * offset, and size come from the kernel symbol table.
- *
- * If a kernel symbol happens to fall at the end of a message close
- * in length to LOG_LINE_LENGTH, the symbol will not be expanded.
- * (This should never happen, since the kernel should never generate
- * messages that long.
  */
 static void LogLine(char *ptr, int len)
 {
     enum parse_state_enum {
- PARSING_TEXT,
- PARSING_SYMSTART, /* at < */
- PARSING_SYMBOL,
- PARSING_SYMEND /* at ] */
+ PARSING_TEXT
     };
 
     static char line_buff[LOG_LINE_LENGTH];
@@ -653,8 +619,6 @@
     static enum parse_state_enum parse_state = PARSING_TEXT;
     static int space = sizeof(line_buff)-1;
 
- static char *sym_start; /* points at the '<' of a symbol */
-
     auto int delta = 0; /* number of chars copied */
 
     while( len > 0 )
@@ -678,165 +642,53 @@
             parse_state = PARSING_TEXT;
         }
 
- switch( parse_state )
- {
- case PARSING_TEXT:
- delta = copyin( line, space, ptr, len, "\n[%" );
- line += delta;
- ptr += delta;
- space -= delta;
- len -= delta;
-
- if( space == 0 || len == 0 )
- {
- break; /* full line_buff or end of input buffer */
- }
-
- if( *ptr == '\n' ) /* newline */
- {
- *line++ = *ptr++; /* copy it in */
- space -= 1;
- len -= 1;
-
- *line = 0; /* force null terminator */
- Syslog( LOG_INFO, line_buff );
- line = line_buff;
- space = sizeof(line_buff)-1;
- break;
- }
- if( *ptr == '[' ) /* possible kernel symbol */
- {
- *line++ = *ptr++;
- space -= 1;
- len -= 1;
- parse_state = PARSING_SYMSTART; /* at < */
- break;
- }
- if( *ptr == '%' ) /* dangerous printf marker */
- {
- delta = 0;
- while (len && *ptr == '%')
- {
- *line++ = *ptr++; /* copy it in */
- space -= 1;
- len -= 1;
- delta++;
- }
- if (delta % 2) /* odd amount of %'s */
- {
- if (space)
- {
- *line++ = '%'; /* so simply add one */
- space -= 1;
- }
- else
- {
- *line++ = '\0'; /* remove the last one / terminate the string */
- }
-
- }
- }
- break;
-
- case PARSING_SYMSTART:
- if( *ptr != '<' )
- {
- parse_state = PARSING_TEXT; /* not a symbol */
- break;
- }
-
- /*
- ** Save this character for now. If this turns out to
- ** be a valid symbol, this char will be replaced later.
- ** If not, we'll just leave it there.
- */
-
- sym_start = line; /* this will point at the '<' */
-
- *line++ = *ptr++;
- space -= 1;
- len -= 1;
- parse_state = PARSING_SYMBOL; /* symbol... */
- break;
-
- case PARSING_SYMBOL:
- delta = copyin( line, space, ptr, len, ">\n[" );
- line += delta;
- ptr += delta;
- space -= delta;
- len -= delta;
- if( space == 0 || len == 0 )
- {
- break; /* full line_buff or end of input buffer */
- }
- if( *ptr != '>' )
- {
- parse_state = PARSING_TEXT;
- break;
- }
-
- *line++ = *ptr++; /* copy the '>' */
- space -= 1;
- len -= 1;
-
- parse_state = PARSING_SYMEND;
-
- break;
-
- case PARSING_SYMEND:
- if( *ptr != ']' )
- {
- parse_state = PARSING_TEXT; /* not a symbol */
- break;
- }
-
- /*
- ** It's really a symbol! Replace address with the
- ** symbol text.
- */
- {
- auto int sym_space;
-
- unsigned long value;
- auto struct symbol sym;
- auto char *symbol;
-
- *(line-1) = 0; /* null terminate the address string */
- value = strtoul(sym_start+1, (char **) 0, 16);
- *(line-1) = '>'; /* put back delim */
-
- symbol = LookupSymbol(value, &sym);
- if ( !symbol_lookup || symbol == (char *) 0 )
- {
- parse_state = PARSING_TEXT;
- break;
- }
-
- /*
- ** verify there is room in the line buffer
- */
- sym_space = space + ( line - sym_start );
- if( sym_space < strlen(symbol) + 30 ) /*(30 should be overkill)*/
- {
- parse_state = PARSING_TEXT; /* not enough space */
- break;
- }
-
- delta = sprintf( sym_start, "%s+%d/%d]",
- symbol, sym.offset, sym.size );
-
- space = sym_space + delta;
- line = sym_start + delta;
- }
- ptr++;
- len--;
- parse_state = PARSING_TEXT;
- break;
-
- default: /* Can't get here! */
- parse_state = PARSING_TEXT;
+ delta = copyin( line, space, ptr, len, "\n%" );
+ line += delta;
+ ptr += delta;
+ space -= delta;
+ len -= delta;
+
+ if( space == 0 || len == 0 )
+ {
+ break; /* full line_buff or end of input buffer */
+ }
+
+ if( *ptr == '\n' ) /* newline */
+ {
+ *line++ = *ptr++; /* copy it in */
+ space -= 1;
+ len -= 1;
+
+ *line = 0; /* force null terminator */
+ Syslog( LOG_INFO, line_buff );
+ line = line_buff;
+ space = sizeof(line_buff)-1;
+ break;
+ }
+ if( *ptr == '%' ) /* dangerous printf marker */
+ {
+ delta = 0;
+ while (len && *ptr == '%')
+ {
+ *line++ = *ptr++; /* copy it in */
+ space -= 1;
+ len -= 1;
+ delta++;
+ }
+ if (delta % 2) /* odd amount of %'s */
+ {
+ if (space)
+ {
+ *line++ = '%'; /* so simply add one */
+ space -= 1;
+ }
+ else
+ {
+ *line++ = '\0'; /* remove the last one / terminate the string */
+ }
 
- }
+ }
+ }
     }
 
     return;
@@ -911,7 +763,7 @@
         chdir ("/");
 #endif
         /* Parse the command-line. */
- while ((ch = getopt(argc, argv, "c:df:iIk:nopsvx")) != EOF)
+ while ((ch = getopt(argc, argv, "c:df:nosv")) != EOF)
                 switch((char)ch)
                 {
                     case 'c': /* Set console message level. */
@@ -924,33 +776,18 @@
                         output = optarg;
                         use_output++;
                         break;
- case 'i': /* Reload module symbols. */
- SignalDaemon(SIGUSR1);
- return(0);
- case 'I':
- SignalDaemon(SIGUSR2);
- return(0);
- case 'k': /* Kernel symbol file. */
- symfile = optarg;
- break;
                     case 'n': /* don't fork */
                         no_fork++;
                         break;
                     case 'o': /* One-shot mode. */
                         one_shot = 1;
                         break;
- case 'p':
- SetParanoiaLevel(1); /* Load symbols on oops. */
- break;
                     case 's': /* Use syscall interface. */
                         use_syscall = 1;
                         break;
                     case 'v':
                         printf("klogd %s-%s\n", VERSION, PATCHLEVEL);
                         exit (1);
- case 'x':
- symbol_lookup = 0;
- break;
                 }
 
 
@@ -1056,9 +893,6 @@
         /* Handle one-shot logging. */
         if ( one_shot )
         {
- if (symbol_lookup) {
- InitKsyms(symfile);
- }
                 if ( (logsrc = GetKernelLogSrc()) == kernel )
                         LogKernelLine();
                 else
@@ -1071,9 +905,6 @@
         sleep(KLOGD_DELAY);
 #endif
         logsrc = GetKernelLogSrc();
- if (symbol_lookup) {
- InitKsyms(symfile);
- }
 
         /* The main loop. */
         while (1)
diff -Nru a/src/sysklogd-1.3-31/klogd.h b/src/sysklogd-1.3-31/klogd.h
--- a/src/sysklogd-1.3-31/klogd.h Mon Dec 11 19:32:23 2000
+++ b/src/sysklogd-1.3-31/klogd.h Mon Dec 11 19:32:23 2000
@@ -33,8 +33,4 @@
 
 
 /* Function prototypes. */
-extern int InitKsyms(char *);
-extern int InitMsyms(void);
-extern char * ExpandKadds(char *, char *);
-extern void SetParanoiaLevel(int);
 extern void Syslog(int priority, char *fmt, ...);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Dec 15 2000 - 21:00:23 EST