Re: [opennms-devel] [RFC] memory related fixes for mib2opennms
Riccardo Magliocchetti <[email protected]>
| Newsgroups | gmane.network.opennms.bugs |
|---|---|
| Message-ID | <484542D6.8070407__26318.5916388922$1212498753$gmane$org@gmail.com> |
[added to cc alex benee because he found and fixed the same off by one] Riccardo Magliocchetti wrote: > hi, > > the attached patch fix a couple of leaks, check for malloc() failures > and properly call smiExit() before exiting. > > The remaining leak and two of three errors appears to be in libsmi, the > only error is mib2opennms is this: > > ==8184== Invalid write of size 1 > ==8184== at 0x40231D7: strcat (mc_replace_strmem.c:186) > ==8184== by 0x804940F: main (mib2opennms.c:261) > ==8184== Address 0x41c4196 is 0 bytes after a block of size 22 alloc'd > ==8184== at 0x4021AB8: malloc (vg_replace_malloc.c:207) > ==8184== by 0x80493C6: main (mib2opennms.c:249) > > It's an off by one somewhere near the strlen but i have to check more > carefully. Rediffed the patch with this changelog: - fixed the off by one, this fixed the three read / write error reported by memcheck - removed the exit in case of file open error in order to avoid the leaks - readded the removal of unneeded initialization in order to keep the diff smaller, will redo later If you agree i'd like to post this patch to bug 2501 which alex already opened, after changing the title to reflect the broader scope. thanks, rm ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Please read the OpenNMS Mailing List FAQ: http://www.opennms.org/index.php/Mailing_List_FAQ opennms-devel mailing list To *unsubscribe* or change your subscription options, see the bottom of this page: https://lists.sourceforge.net/lists/listinfo/opennms-devel
mib2opennms-plug-leaks-check-malloc-v3.diff
(text/x-diff, 2.4 KB)
Index: mib2opennms.c
===================================================================
--- mib2opennms.c (revisione 9249)
+++ mib2opennms.c (copia locale)
@@ -92,7 +92,7 @@
return output;
}
-void dumpXml(SmiModule* smiModule, FILE* file, EventDefaults* defs) {
+int dumpXml(SmiModule* smiModule, FILE* file, EventDefaults* defs) {
SmiNode* smiNode;
SmiNode* tmpNode;
SmiElement* smiElem;
@@ -133,6 +133,10 @@
fprintf(file, "<table>");
logmsg = (char *) malloc( 2000 * sizeof (char));
+ if (!logmsg) {
+ perror("Cannot allocate memory");
+ return 0;
+ }
logmsg[0]='\0';
sprintf(logmsg, "\t\t<logmsg dest='logndisplay'><p>\n\t\t\t%s trap received ",
smiNode->name);
@@ -176,6 +180,8 @@
}
fprintf(file, "<!-- End of auto generated data from MIB: %s -->\n",
smiModule->name);
+
+ return 1;
}
void usage() {
@@ -238,21 +244,30 @@
smiInit(NULL);
- pathlen = strlen(STANDARD_PATH) + (mibpath == NULL ? 0 : strlen(mibpath)+1 /* for the colon */);
+ pathlen = strlen(STANDARD_PATH);
+ /* Add enough space for colon and null termination */
+ pathlen += (mibpath == NULL ? 1 : strlen(mibpath) + 2);
newpath = (char *) malloc( pathlen * sizeof(char) );
+ if (!newpath) {
+ perror("Cannot allocate memory");
+ goto out;
+ }
newpath[0] = '\0';
if (mibpath != NULL) {
strcat(newpath, mibpath);
strcat(newpath, ":");
}
-
strcat(newpath, STANDARD_PATH);
smiSetPath(newpath);
modules = (SmiModule **) malloc( argc * sizeof(SmiModule *));
+ if (!modules) {
+ perror("Cannot allocate memory");
+ goto out1;
+ }
moduleCount = 0;
while( optind < argc ) {
@@ -282,26 +297,36 @@
file = fopen(filename, "w");
if ( file == NULL ) {
perror("Could not open file for writing");
- exit(1);
+ goto out2;
}
}
defaults = (EventDefaults*) malloc(sizeof(struct EventDefaults));
+ if (!defaults) {
+ perror("Cannot allocate memory");
+ goto out2;
+ }
defaults->ueiPrefix = "uei.opennms.org/mib2opennms/";
defaults->severity = "Indeterminate";
for ( i = 0; i < moduleCount; i++ ) {
smiModule = modules[i];
verbose(3, "Dumping %s to file\n", smiModule->name);
- dumpXml(smiModule, file, defaults);
+ if (!dumpXml(smiModule, file, defaults))
+ break;
}
-
+
+ free(defaults);
+out2:
+ free(modules);
+out1:
+ free(newpath);
+out:
if ( filename != NULL ) {
fflush(file);
fclose(file);
}
+ smiExit();
- free(defaults);
-
exit(0);
}