From 769598da4cfc7c9fb3ceb337044a9313e4b1b68d Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 8 Apr 2008 14:40:19 +0200 Subject: cleanup and bugfix in imklog - some cleanup in imklog - bugfix: potential segfault in imklog when kernel is compiled without /proc/kallsyms and the file System.map is missing. Thanks to Andrea Morandi for pointing it out and suggesting a fix. --- ChangeLog | 4 ++ plugins/imklog/imklog.h | 3 +- plugins/imklog/ksym.c | 128 +++++++++++++++------------------------------- plugins/imklog/ksym_mod.c | 93 ++++++++++++--------------------- plugins/imklog/ksyms.h | 15 +++--- plugins/imklog/module.h | 9 ++-- 6 files changed, 91 insertions(+), 161 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2e1b28e0..8fda865b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,10 @@ Version 3.14.2 (rgerhards), 2008-04-?? Vrabec for patching it based on the development in sysklogd - and thanks to the sysklogd project for upgrading klogd to support the new functionality +- some cleanup in imklog +- bugfix: potential segfault in imklog when kernel is compiled without + /proc/kallsyms and the file System.map is missing. Thanks to + Andrea Morandi for pointing it out and suggesting a fix. --------------------------------------------------------------------------- Version 3.14.1 (rgerhards), 2008-04-04 - bugfix: some messages were emited without hostname diff --git a/plugins/imklog/imklog.h b/plugins/imklog/imklog.h index 2db75009..71525a79 100644 --- a/plugins/imklog/imklog.h +++ b/plugins/imklog/imklog.h @@ -42,6 +42,5 @@ extern void vsyslog(int pri, const char *fmt, va_list ap); rsRetVal Syslog(int priority, char *fmt, ...) __attribute__((format(printf,2, 3))); #endif /* #ifndef IMKLOG_H_INCLUDED */ -/* - * vi:set ai: +/* vi:set ai: */ diff --git a/plugins/imklog/ksym.c b/plugins/imklog/ksym.c index 4fa2fbb6..b7d5903e 100644 --- a/plugins/imklog/ksym.c +++ b/plugins/imklog/ksym.c @@ -1,8 +1,9 @@ -/* - ksym.c - functions for kernel address->symbol translation - Copyright (c) 1995, 1996 Dr. G.W. Wettstein - Copyright (c) 1996 Enjellic Systems Development - +/* ksym.c - functions for kernel address->symbol translation + * Copyright (c) 1995, 1996 Dr. G.W. Wettstein + * Copyright (c) 1996 Enjellic Systems Development + * Copyright (c) 1998-2007 Martin Schulze + * Copyright (C) 2007-2008 Rainer Gerhards + * * This file is part of rsyslog. * * Rsyslog is free software: you can redistribute it and/or modify @@ -181,26 +182,20 @@ extern int InitKsyms(char *mapfile) /* * Search for and open the file containing the kernel symbols. */ - if ( mapfile != (char *) 0 ) - { + if ( mapfile != (char *) 0 ) { if ( (sym_file = fopen(mapfile, "r")) == (FILE *) 0 ) { - Syslog(LOG_WARNING, "Cannot open map file: %s.", \ - mapfile); + Syslog(LOG_WARNING, "Cannot open map file: %s.", mapfile); return(0); } - } - else - { - if ( (mapfile = FindSymbolFile()) == (char *) 0 ) - { + } else { + if ( (mapfile = FindSymbolFile()) == (char *) 0 ) { Syslog(LOG_WARNING, "Cannot find map file."); dbgprintf("Cannot find map file.\n"); return(0); } - if ( (sym_file = fopen(mapfile, "r")) == (FILE *) 0 ) - { + if ( (sym_file = fopen(mapfile, "r")) == (FILE *) 0 ) { Syslog(LOG_WARNING, "Cannot open map file."); dbgprintf("Cannot open map file.\n"); return(0); @@ -216,11 +211,8 @@ extern int InitKsyms(char *mapfile) * e-mail me a diff containing a parser with suitable political * correctness -- GW. */ - while ( !feof(sym_file) ) - { - if ( fscanf(sym_file, "%lx %c %s\n", &address, &type, sym) - != 3 ) - { + while ( !feof(sym_file) ) { + if ( fscanf(sym_file, "%lx %c %s\n", &address, &type, sym) != 3 ) { Syslog(LOG_ERR, "Error in symbol table input (#1)."); fclose(sym_file); return(0); @@ -228,8 +220,7 @@ extern int InitKsyms(char *mapfile) if(dbgPrintSymbols) dbgprintf("Address: %lx, Type: %c, Symbol: %s\n", address, type, sym); - if ( AddSymbol(address, sym) == 0 ) - { + if ( AddSymbol(address, sym) == 0 ) { Syslog(LOG_ERR, "Error adding symbol - %s.", sym); fclose(sym_file); return(0); @@ -241,16 +232,14 @@ extern int InitKsyms(char *mapfile) Syslog(LOG_INFO, "Loaded %d symbols from %s.", num_syms, mapfile); - switch ( version ) - { + switch(version) { case -1: Syslog(LOG_WARNING, "Symbols do not match kernel version."); num_syms = 0; break; case 0: - Syslog(LOG_WARNING, "Cannot verify that symbols match " \ - "kernel version."); + Syslog(LOG_WARNING, "Cannot verify that symbols match kernel version."); break; case 1: @@ -311,18 +300,16 @@ static char *FindSymbolFile(void) auto FILE *sym_file = (FILE *) 0; - if ( uname(&utsname) < 0 ) - { + if ( uname(&utsname) < 0 ) { Syslog(LOG_ERR, "Cannot get kernel version information."); return(0); } dbgprintf("Searching for symbol map.\n"); - for (mf = system_maps; *mf != (char *) 0 && file == (char *) 0; ++mf) - { + for(mf = system_maps; *mf != (char *) 0 && file == (char *) 0; ++mf) { - sprintf (symfile, "%s-%s", *mf, utsname.release); + snprintf(symfile, sizeof(symfile), "%s-%s", *mf, utsname.release); dbgprintf("Trying %s.\n", symfile); if ( (sym_file = fopen(symfile, "r")) != (FILE *) 0 ) { if (CheckMapVersion(symfile) == 1) @@ -341,10 +328,7 @@ static char *FindSymbolFile(void) } - /* - * At this stage of the game we are at the end of the symbol - * tables. - */ + /* At this stage of the game we are at the end of the symbol tables. */ dbgprintf("End of search list encountered.\n"); return(file); } @@ -407,8 +391,7 @@ static int CheckVersion(char *version) return(0); - /* - * Since the symbol looks like a kernel version we can start + /* Since the symbol looks like a kernel version we can start * things out by decoding the version string into its component * parts. */ @@ -420,24 +403,20 @@ static int CheckVersion(char *version) strlen(prefix), major, minor, patch); sprintf(vstring, "%d.%d.%d", major, minor, patch); - /* - * We should now have the version string in the vstring variable in + /* We should now have the version string in the vstring variable in * the same format that it is stored in by the kernel. We now * ask the kernel for its version information and compare the two * values to determine if our system map matches the kernel * version level. */ - if ( uname(&utsname) < 0 ) - { + if ( uname(&utsname) < 0 ) { Syslog(LOG_ERR, "Cannot get kernel version information."); return(0); } dbgprintf("Comparing kernel %s with symbol table %s.\n", utsname.release, vstring); - if ( sscanf (utsname.release, "%d.%d.%d", &major, &minor, &patch) < 3 ) - { - Syslog(LOG_ERR, "Kernel send bogus release string `%s'.", - utsname.release); + if ( sscanf (utsname.release, "%d.%d.%d", &major, &minor, &patch) < 3 ) { + Syslog(LOG_ERR, "Kernel send bogus release string `%s'.", utsname.release); return(0); } @@ -494,11 +473,8 @@ static int CheckMapVersion(char *fname) Syslog(LOG_INFO, "Inspecting %s", fname); version = 0; - while ( !feof(sym_file) && (version == 0) ) - { - if ( fscanf(sym_file, "%lx %c %s\n", &address, \ - &type, sym) != 3 ) - { + while ( !feof(sym_file) && (version == 0) ) { + if ( fscanf(sym_file, "%lx %c %s\n", &address, &type, sym) != 3 ) { Syslog(LOG_ERR, "Error in symbol table input (#2)."); fclose(sym_file); return(0); @@ -509,11 +485,9 @@ static int CheckMapVersion(char *fname) } fclose(sym_file); - switch ( version ) - { + switch ( version ) { case -1: - Syslog(LOG_ERR, "Symbol table has incorrect " \ - "version number.\n"); + Syslog(LOG_ERR, "Symbol table has incorrect version number.\n"); break; case 0: dbgprintf("No version information found.\n"); @@ -546,14 +520,13 @@ static int CheckMapVersion(char *fname) static int AddSymbol(unsigned long address, char *symbol) { /* Allocate the the symbol table entry. */ - sym_array = (struct sym_table *) realloc(sym_array, (num_syms+1) * \ + sym_array = (struct sym_table *) realloc(sym_array, (num_syms+1) * sizeof(struct sym_table)); if ( sym_array == (struct sym_table *) 0 ) return(0); /* Then the space for the symbol. */ - sym_array[num_syms].name = (char *) malloc(strlen(symbol)*sizeof(char)\ - + 1); + sym_array[num_syms].name = (char *) malloc(strlen(symbol)*sizeof(char) + 1); if ( sym_array[num_syms].name == (char *) 0 ) return(0); @@ -583,12 +556,7 @@ static int AddSymbol(unsigned long address, char *symbol) * If a match is found the pointer to the symbolic name most * closely matching the address is returned. **************************************************************************/ -char * LookupSymbol(value, sym) - - unsigned long value; - - struct symbol *sym; - +char * LookupSymbol(unsigned long value, struct symbol *sym) { auto int lp; @@ -606,10 +574,8 @@ char * LookupSymbol(value, sym) if ( value < sym_array[0].value ) return((char *) 0); - for(lp = 0; lp <= num_syms; ++lp) - { - if ( sym_array[lp].value > value ) - { + for(lp = 0; lp <= num_syms; ++lp) { + if ( sym_array[lp].value > value ) { ksym.offset = value - sym_array[lp-1].value; ksym.size = sym_array[lp].value - \ sym_array[lp-1].value; @@ -620,20 +586,16 @@ char * LookupSymbol(value, sym) name = LookupModuleSymbol(value, &msym); - if ( ksym.offset == 0 && msym.offset == 0 ) - { + if ( ksym.offset == 0 && msym.offset == 0 ) { return((char *) 0); } if ( ksym.offset == 0 || msym.offset < 0 || - (ksym.offset > 0 && ksym.offset < msym.offset) ) - { + (ksym.offset > 0 && ksym.offset < msym.offset) ) { sym->offset = ksym.offset; sym->size = ksym.size; return(last); - } - else - { + } else { sym->offset = msym.offset; sym->size = msym.size; return(name); @@ -730,12 +692,10 @@ extern char *ExpandKadds(char *line, char *el) * messages in this line. */ if ( (num_syms == 0) || - (kp = strstr(line, "[<")) == (char *) 0 ) - { + (kp = strstr(line, "[<")) == (char *) 0 ) { #ifdef __sparc__ if (num_syms) { - /* - * On SPARC, register dumps do not have the [< >] characters in it. + /* On SPARC, register dumps do not have the [< >] characters in it. */ static struct sparc_tests { char *str; @@ -815,14 +775,12 @@ extern char *ExpandKadds(char *line, char *el) } /* Loop through and expand all kernel messages. */ - do - { + do { while ( sl < kp+1 ) *elp++ = *sl++; /* Now poised at a kernel delimiter. */ - if ( (kp = strstr(sl, ">]")) == (char *) 0 ) - { + if ( (kp = strstr(sl, ">]")) == (char *) 0 ) { strcpy(el, sl); return(el); } @@ -839,8 +797,7 @@ extern char *ExpandKadds(char *line, char *el) (sym.size==0) ? symbol+1 : symbol, sym.offset, sym.size); value = 2; - if ( sym.size != 0 ) - { + if ( sym.size != 0 ) { --value; ++kp; elp += sprintf(elp, "+0x%x/0x%02x", sym.offset, sym.size); @@ -871,7 +828,6 @@ extern char *ExpandKadds(char *line, char *el) * present when resolving kernel exceptions. * Return: void **************************************************************************/ - extern void SetParanoiaLevel(int level) { i_am_paranoid = level; diff --git a/plugins/imklog/ksym_mod.c b/plugins/imklog/ksym_mod.c index ec1231be..11535a5f 100644 --- a/plugins/imklog/ksym_mod.c +++ b/plugins/imklog/ksym_mod.c @@ -1,8 +1,10 @@ /* - ksym_mod.c - functions for building symbol lookup tables for klogd - Copyright (c) 1995, 1996 Dr. G.W. Wettstein - Copyright (c) 1996 Enjellic Systems Development - + * ksym_mod.c - functions for building symbol lookup tables for klogd + * Copyright (c) 1995, 1996 Dr. G.W. Wettstein + * Copyright (c) 1996 Enjellic Systems Development + * Copyright (c) 1998-2007 Martin Schulze + * Copyright (C) 2007-2008 Rainer Gerhards + * * This file is part of rsyslog. * * Rsyslog is free software: you can redistribute it and/or modify @@ -145,9 +147,7 @@ extern int InitMsyms(void) auto int rtn, tmp; - FILE *ksyms; - char buf[128]; char *p; @@ -156,8 +156,7 @@ extern int InitMsyms(void) ksyms = fopen(KSYMS, "r"); - if ( ksyms == NULL ) - { + if ( ksyms == NULL ) { if ( errno == ENOENT ) Syslog(LOG_INFO, "No module symbols loaded - " "kernel modules not enabled.\n"); @@ -170,8 +169,7 @@ extern int InitMsyms(void) dbgprintf("Loading kernel module symbols - Source: %s\n", KSYMS); - while ( fgets(buf, sizeof(buf), ksyms) != NULL ) - { + while ( fgets(buf, sizeof(buf), ksyms) != NULL ) { if (num_syms > 0 && index(buf, '[') == NULL) continue; @@ -187,13 +185,13 @@ extern int InitMsyms(void) AddSymbol(buf); } - fclose(ksyms); + if(ksyms != NULL) + fclose(ksyms); have_modules = 1; /* Sort the symbol tables in each module. */ - for (rtn = tmp = 0; tmp < num_modules; ++tmp) - { + for (rtn = tmp = 0; tmp < num_modules; ++tmp) { rtn += sym_array_modules[tmp].num_syms; if ( sym_array_modules[tmp].num_syms < 2 ) continue; @@ -243,14 +241,11 @@ extern void DeinitMsyms(void) * Return: void **************************************************************************/ static void FreeModules() - { auto int nmods, nsyms; - auto struct Module *mp; - /* Check to see if the module symbol tables need to be cleared. */ have_modules = 0; if ( num_modules == 0 ) @@ -259,8 +254,7 @@ static void FreeModules() if ( sym_array_modules == NULL ) return; - for (nmods = 0; nmods < num_modules; ++nmods) - { + for (nmods = 0; nmods < num_modules; ++nmods) { mp = &sym_array_modules[nmods]; if ( mp->num_syms == 0 ) continue; @@ -278,28 +272,26 @@ static void FreeModules() return; } + /************************************************************************** - * * Function: AddModule - * * - * * Purpose: This function is responsible for adding a module to - * * the list of currently loaded modules. - * * - * * Arguments: (const char *) module - * * - * * module:-> The name of the module. - * * - * * Return: struct Module * - * **************************************************************************/ + * Function: AddModule + * + * Purpose: This function is responsible for adding a module to + * the list of currently loaded modules. + * + * Arguments: (const char *) module + * + * module:-> The name of the module. + * + * Return: struct Module * + **************************************************************************/ struct Module *AddModule(module) - const char *module; - { struct Module *mp; - if ( num_modules == 0 ) - { + if ( num_modules == 0 ) { sym_array_modules = (struct Module *)malloc(sizeof(struct Module)); if ( sym_array_modules == NULL ) @@ -308,9 +300,7 @@ struct Module *AddModule(module) return NULL; } mp = sym_array_modules; - } - else - { + } else { /* Allocate space for the module. */ mp = (struct Module *) \ realloc(sym_array_modules, \ @@ -353,9 +343,7 @@ struct Module *AddModule(module) * successful. False if not. **************************************************************************/ static int AddSymbol(line) - const char *line; - { char *module; unsigned long address; @@ -365,16 +353,13 @@ static int AddSymbol(line) module = index(line, '['); - if ( module != NULL ) - { + if ( module != NULL ) { p = index(module, ']'); - if ( p != NULL ) *p = '\0'; - p = module++; - - while ( isspace(*(--p)) ); + while ( isspace(*(--p)) ) + /*SKIP*/; *(++p) = '\0'; } @@ -392,14 +377,12 @@ static int AddSymbol(line) if ( num_modules == 0 || ( lastmodule == NULL && module != NULL ) || ( module == NULL && lastmodule != NULL) || - ( module != NULL && strcmp(module, lastmodule))) - { + ( module != NULL && strcmp(module, lastmodule))) { mp = AddModule(module); if ( mp == NULL ) return(0); - } - else + } else mp = &sym_array_modules[num_modules-1]; lastmodule = mp->name; @@ -444,29 +427,21 @@ static int AddSymbol(line) * closely matching the address is returned. **************************************************************************/ extern char * LookupModuleSymbol(value, sym) - unsigned long value; - struct symbol *sym; - { auto int nmod, nsym; - auto struct sym_table *last; - auto struct Module *mp; - static char ret[100]; - sym->size = 0; sym->offset = 0; if ( num_modules == 0 ) return((char *) 0); - for (nmod = 0; nmod < num_modules; ++nmod) - { + for (nmod = 0; nmod < num_modules; ++nmod) { mp = &sym_array_modules[nmod]; /* @@ -475,8 +450,7 @@ extern char * LookupModuleSymbol(value, sym) */ for(nsym = 1, last = &mp->sym_array[0]; nsym < mp->num_syms; - ++nsym) - { + ++nsym) { if ( mp->sym_array[nsym].value > value ) { if ( sym->size == 0 || @@ -507,4 +481,3 @@ extern char * LookupModuleSymbol(value, sym) /* It has been a hopeless exercise. */ return((char *) 0); } - diff --git a/plugins/imklog/ksyms.h b/plugins/imklog/ksyms.h index 316950a0..b5362ff3 100644 --- a/plugins/imklog/ksyms.h +++ b/plugins/imklog/ksyms.h @@ -1,10 +1,9 @@ -/* - ksym.h - Definitions for symbol table utilities. - Copyright (c) 1995, 1996 Dr. G.W. Wettstein - Copyright (c) 1996 Enjellic Systems Development - - This file is part of the sysklogd package, a kernel and system log daemon. - +/* ksym.h - Definitions for symbol table utilities. + * Copyright (c) 1995, 1996 Dr. G.W. Wettstein + * Copyright (c) 1996 Enjellic Systems Development + * Copyright (c) 2004-7 Martin Schulze + * Copyright (c) 2007-2008 Rainer Gerhards + * * This file is part of rsyslog. * * Rsyslog is free software: you can redistribute it and/or modify @@ -21,7 +20,7 @@ * along with Rsyslog. If not, see . * * A copy of the GPL can be found in the file "COPYING" in this distribution. -*/ + */ /* Variables, structures and type definitions static to this module. */ diff --git a/plugins/imklog/module.h b/plugins/imklog/module.h index 7a26ad02..38a26fea 100644 --- a/plugins/imklog/module.h +++ b/plugins/imklog/module.h @@ -1,6 +1,7 @@ -/* Module definitions for klogd's module support - * - * Copyright 2007 by Rainer Gerhards and others +/* module.h - Miscellaneous module definitions + * Copyright (c) 1996 Richard Henderson + * Copyright (c) 2004-7 Martin Schulze + * Copyright (c) 2007-2008 Rainer Gerhards * * This file is part of rsyslog. * @@ -19,7 +20,6 @@ * * A copy of the GPL can be found in the file "COPYING" in this distribution. */ - struct sym_table { unsigned long value; @@ -33,4 +33,3 @@ struct Module char *name; }; - -- cgit