[PATCH] cpio hardlink support and possible memory leak fix

[Luciano Miguel Ferreira Rocha]

Hello,

Attached is a patch that adds hard-link support for busybox's cpio.

I tested it with /usr/share/zoneinfo and it worked fine (both data,
hardlinks, permissions and owner/group).

While adding it, I noticed that file_header->name was allocated but
never freed:
archival/libunarchive/get_header_cpio.c +77
       file_header->name = (char *) xzalloc(namesize + 1);

As such, in the patch I send, I reuse it when creating hard links, and
add a free(file_header->name) before the allocation. That means that
there's no longer a memory leak for each filename (iff I'm reading the
code correctly), but the memory for the last filename, the file_header and
archive_handle is still allocated when cpio_main returns. As I don't
know if busybox has a mode to execute internal commands without an exec,
I don't know if that is serious.

Regards,
Luciano Rocha

-- 
lfr
0/0
-------------- next part --------------
diff -ur busybox-1.2.1.orig/archival/libunarchive/get_header_cpio.c busybox-1.2.1/archival/libunarchive/get_header_cpio.c
--- busybox-1.2.1.orig/archival/libunarchive/get_header_cpio.c	2006-06-30 23:42:03.000000000 +0100
+++ busybox-1.2.1/archival/libunarchive/get_header_cpio.c	2006-09-21 01:16:58.000000000 +0100
@@ -12,7 +12,7 @@
 #include "libbb.h"
 
 typedef struct hardlinks_s {
-	file_header_t *entry;
+	char *name;
 	int inode;
 	struct hardlinks_s *next;
 } hardlinks_t;
@@ -21,35 +21,52 @@
 {
 	static hardlinks_t *saved_hardlinks = NULL;
 	static unsigned short pending_hardlinks = 0;
+	static int inode;
 	file_header_t *file_header = archive_handle->file_header;
 	char cpio_header[110];
 	int namesize;
 	char dummy[16];
-	int major, minor, nlink, inode;
+	int major, minor, nlink;
 
 	if (pending_hardlinks) { /* Deal with any pending hardlinks */
-		hardlinks_t *tmp;
-		hardlinks_t *oldtmp;
+		hardlinks_t *tmp, *oldtmp;
 
 		tmp = saved_hardlinks;
 		oldtmp = NULL;
 
+		file_header->link_name = file_header->name;
+		file_header->size = 0;
+
 		while (tmp) {
-			bb_error_msg_and_die("need to fix this\n");
-			if (tmp->entry->link_name) { /* Found a hardlink ready to be extracted */
-				file_header = tmp->entry;
-				if (oldtmp) {
-					oldtmp->next = tmp->next; /* Remove item from linked list */
-				} else {
-					saved_hardlinks = tmp->next;
-				}
-				free(tmp);
+			if (tmp->inode != inode) {
+				tmp = tmp->next;
 				continue;
 			}
+
+			file_header->name = tmp->name;
+
+			if (archive_handle->filter(archive_handle) == EXIT_SUCCESS) {
+				archive_handle->action_data(archive_handle);
+				archive_handle->action_header(archive_handle->file_header);
+			}
+
+			pending_hardlinks--;
+
 			oldtmp = tmp;
 			tmp = tmp->next;
+			free (oldtmp->name);
+			free (oldtmp);
+			if (oldtmp == saved_hardlinks) saved_hardlinks = tmp;
+		}
+
+		file_header->name = file_header->link_name;
+
+		if (pending_hardlinks > 1) {
+			bb_error_msg("error resolving hardlink: did you create the archive with GNU cpio 2.0-2.2?");
 		}
-		pending_hardlinks = 0; /* No more pending hardlinks, read next file entry */
+
+		/* No more pending hardlinks, read next file entry */
+		pending_hardlinks = 0;
 	}
 
 	/* There can be padding before archive header */
@@ -74,6 +91,7 @@
 	    file_header->size = tmpsize;
 	}
 
+	free(file_header->name);
 	file_header->name = (char *) xzalloc(namesize + 1);
 	archive_xread_all(archive_handle, file_header->name, namesize); /* Read in filename */
 	archive_handle->offset += namesize;
@@ -87,11 +105,10 @@
 			hardlinks_t *tmp = saved_hardlinks;
 			hardlinks_t *oldtmp = NULL;
 			while (tmp) {
-				bb_error_msg("%s not created: cannot resolve hardlink", tmp->entry->name);
+				bb_error_msg("%s not created: cannot resolve hardlink", tmp->name);
 				oldtmp = tmp;
 				tmp = tmp->next;
-				free (oldtmp->entry->name);
-				free (oldtmp->entry);
+				free (oldtmp->name);
 				free (oldtmp);
 			}
 			saved_hardlinks = NULL;
@@ -113,22 +130,13 @@
 			hardlinks_t *new = xmalloc(sizeof(hardlinks_t));
 			new->next = saved_hardlinks;
 			new->inode = inode;
-			new->entry = file_header;
+			/* name current allocated, freed later */
+			new->name = file_header->name;
+			file_header->name = NULL;
 			saved_hardlinks = new;
 			return(EXIT_SUCCESS); // Skip this one
 		} else { /* Found the file with data in */
-			hardlinks_t *tmp = saved_hardlinks;
-			pending_hardlinks = 1;
-			while (tmp) {
-				if (tmp->inode == inode) {
-					tmp->entry->link_name = bb_xstrdup(file_header->name);
-					nlink--;
-				}
-				tmp = tmp->next;
-			}
-			if (nlink > 1) {
-				bb_error_msg("error resolving hardlink: did you create the archive with GNU cpio 2.0-2.2?");
-			}
+			pending_hardlinks = nlink;
 		}
 	}
 	file_header->device = makedev(major, minor);
diff -ur busybox-1.2.1.orig/archival/libunarchive/init_handle.c busybox-1.2.1/archival/libunarchive/init_handle.c
--- busybox-1.2.1.orig/archival/libunarchive/init_handle.c	2006-06-30 23:42:03.000000000 +0100
+++ busybox-1.2.1/archival/libunarchive/init_handle.c	2006-09-21 00:57:28.000000000 +0100
@@ -26,6 +26,7 @@
 	/* Initialise default values */
 	archive_handle = xzalloc(sizeof(archive_handle_t));
 	archive_handle->file_header = xmalloc(sizeof(file_header_t));
+	archive_handle->file_header->name = NULL;
 	archive_handle->action_header = header_skip;
 	archive_handle->action_data = data_skip;
 	archive_handle->filter = filter_accept_all;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20061015/f2bbbdaf/attachment.pgp