Clean up some memory management in HTTP/FTP file transferring. Fix loader

segfaults caused by improperly appending the MAC address-specific HTTP
headers (#328191).

3 files changed, 41 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index 29c25d04b..82779c818 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -8,6 +8,12 @@
 
 	* lang-table:  Fix lang setting for Romanian (#327431).
 
+	* loader2/urls.c (urlinstStartTransfer):  Don't be so sloppy with
+	freeing memory.
+	* loader2/urlinstall.c (getFileFromUrl):  Likewise.  Also clean up
+	code that appends MAC address-specific HTTP headers to fix segfaults
+	(#328191).
+
 2007-10-12  Jeremy Katz  <katzj@redhat.com>
 
 	* loader2/hardware.c (detectHardware): Revert ssb hack as kudzu
diff --git a/loader2/urlinstall.c b/loader2/urlinstall.c
index fc6fba582..c6c5030e5 100644
--- a/loader2/urlinstall.c
+++ b/loader2/urlinstall.c
@@ -345,6 +345,7 @@ char * mountUrlImage(struct installMethod * method,
 
 int getFileFromUrl(char * url, char * dest, 
                    struct loaderData_s * loaderData) {
+    int retval = 0;
     char ret[47];
     struct iurlinfo ui;
     enum urlprotocol_t proto = 
@@ -401,56 +402,57 @@ int getFileFromUrl(char * url, char * dest,
 
     if (proto == URL_METHOD_HTTP && FL_KICKSTART_SEND_MAC(flags)) {
         /* find all ethernet devices and make a header entry for each one */
-        int i;
-        unsigned int hdrlen;
-        char *dev, *mac, tmpstr[128];
-        struct device ** devices;
+        int i, q;
+        char *dev, *mac, *tmpstr;
+        struct device **devices;
 
-        hdrlen = 0;
         devices = probeDevices(CLASS_NETWORK, BUS_UNSPEC, PROBE_LOADED);
         for (i = 0; devices && devices[i]; i++) {
             dev = devices[i]->device;
             mac = netlink_interfaces_mac2str(dev);
 
             if (mac) {
-                snprintf(tmpstr, sizeof(tmpstr),
-                         "X-RHN-Provisioning-MAC-%d: %s %s\r\n", i, dev, mac);
-                free(mac);
+                q = asprintf(&tmpstr, "X-RHN-Provisioning-MAC-%d: %s %s\r\n",
+                             i, dev, mac);
 
                 if (!ehdrs) {
-                    hdrlen = 128;
-                    ehdrs = (char *) malloc(hdrlen);
-                    *ehdrs = '\0';
-                } else if ( strlen(tmpstr) + strlen(ehdrs) + 2 > hdrlen) {
-                    hdrlen += 128;
-                    ehdrs = (char *) realloc(ehdrs, hdrlen);
+                    ehdrs = strdup(tmpstr);
+                } else {
+                    ehdrs = (char *) realloc(ehdrs, strlen(ehdrs)+strlen(tmpstr)+1);
+                    strcat(ehdrs, tmpstr);
                 }
 
-                strcat(ehdrs, tmpstr);
+                free(mac);
+                free(tmpstr);
             }
         }
     }
-	
-    fd = urlinstStartTransfer(&ui, file, ehdrs);
+
+    fd = urlinstStartTransfer(&ui, file, NULL);
     if (fd < 0) {
         logMessage(ERROR, "failed to retrieve http://%s/%s%s", ui.address, ui.prefix, file);
-        if (ehdrs) free(ehdrs);
-        return 1;
+        retval = 1;
+        goto err;
     }
-           
+
     rc = copyFileFd(fd, dest);
     if (rc) {
         unlink (dest);
         logMessage(ERROR, "failed to copy file to %s", dest);
-        if (ehdrs) free(ehdrs);
-        return 1;
+        retval = 1;
+        goto err;
     }
 
     urlinstFinishTransfer(&ui, fd);
 
+err:
+    if (file) free(file);
     if (ehdrs) free(ehdrs);
+    if (host) free(host);
+    if (login) free(login);
+    if (password) free(password);
 
-    return 0;
+    return retval;
 }
 
 /* pull kickstart configuration file via http */
diff --git a/loader2/urls.c b/loader2/urls.c
index 7ebb227f8..9194bf377 100644
--- a/loader2/urls.c
+++ b/loader2/urls.c
@@ -181,6 +181,8 @@ int urlinstStartTransfer(struct iurlinfo * ui, char * filename,
     else
         port = atoi(portstr);
 
+    if (portstr) free(portstr);
+
     if (inet_pton(AF_INET, hostname, &addr) >= 1)
         family = AF_INET;
     else if (inet_pton(AF_INET6, hostname, &addr6) >= 1)
@@ -201,24 +203,30 @@ int urlinstStartTransfer(struct iurlinfo * ui, char * filename,
                               ui->login ? ui->login : "anonymous", 
                               ui->password ? ui->password : "rhinstall@", 
                               NULL, port);
-        if (ui->ftpPort < 0)
+        if (ui->ftpPort < 0) {
+            if (hostname) free(hostname);
             return -2;
+        }
 
         fd = ftpGetFileDesc(ui->ftpPort, addr6, family, buf);
         if (fd < 0) {
             close(ui->ftpPort);
+            if (hostname) free(hostname);
             return -1;
         }
     } else {
         fd = httpGetFileDesc(hostname, port, buf, extraHeaders);
-        if (fd < 0)
+        if (fd < 0) {
+            if (portstr) free(portstr);
             return -1;
+        }
     }
 
     if (!FL_CMDLINE(flags))
         winStatus(70, 3, _("Retrieving"), "%s %s...", _("Retrieving"), 
                   filename);
 
+    if (hostname) free(hostname);
     return fd;
 }