>>>>> "KO" == Keith Owens <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
+ *     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 [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/

Reply via email to