[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#657116: libtar: FTBFS on hurd-i386



Source: libtar
Version: 1.2.11-8
Severity: important
Tags: patch
User: debian-hurd@lists.debian.org
Usertags: hurd

Hi,

This patch solves the build problems for GNU/Hurd due to MAXPATHLEN
issues. The solution is to make dynamic string allocations instead of
using fixed length buffers. The functions mkdirhier, basename and
dirname have all been checked with separate test programs on Linux and
Hurd and with valgrind on Linux without problems. Furthermore, parts of
the the code has been reviewed by GN/Hurd developers and Debian GNU/Hurd
developers and maintainers.

Build and run tested on both Linux and Hurd without problems. Also heap
and leak tested on Linux with valgrind. No heap errors, but still memory
leaks (coming from the original code). Removing the memory leaks will
need another deep investigation of the original code, which time does
not permit right now. 

Since there is no upstream, the wish is that the Debian Maintainer
takes a look at this patch and checks if it can be applied. Test
programs are available on request. This patch will affect all
architectures, completely removing the MAXPATHLEN dependencies (except
for a few unused compat functions).

There might be some remaining unneeded header file inclusions, they are
remnants of debugging methods used, both gdb and printf.

Thanks!

diff -ur libtar-1.2.11/compat/basename.c libtar-1.2.11.modified/compat/basename.c
--- libtar-1.2.11/compat/basename.c	2000-10-27 18:16:34.000000000 +0200
+++ libtar-1.2.11.modified/compat/basename.c	2012-01-19 14:20:20.000000000 +0100
@@ -34,17 +34,22 @@
 #include <errno.h>
 #include <string.h>
 #include <sys/param.h>
+#include <stdlib.h>
 
 char *
 openbsd_basename(path)
 	const char *path;
 {
-	static char bname[MAXPATHLEN];
+	static char *bname = NULL;
 	register const char *endp, *startp;
+	int len = 0;
+
+	if (bname != NULL)
+		free(bname);
 
 	/* Empty or NULL string gets treated as "." */
 	if (path == NULL || *path == '\0') {
-		(void)strcpy(bname, ".");
+		bname = strdup(".");
 		return(bname);
 	}
 
@@ -55,7 +60,7 @@
 
 	/* All slashes becomes "/" */
 	if (endp == path && *endp == '/') {
-		(void)strcpy(bname, "/");
+		bname = strdup("/");
 		return(bname);
 	}
 
@@ -64,11 +69,9 @@
 	while (startp > path && *(startp - 1) != '/')
 		startp--;
 
-	if (endp - startp + 1 > sizeof(bname)) {
-		errno = ENAMETOOLONG;
-		return(NULL);
-	}
-	(void)strncpy(bname, startp, endp - startp + 1);
-	bname[endp - startp + 1] = '\0';
+	len = endp - startp + 1;
+	bname = malloc(len + 1);
+	(void)strncpy(bname, startp, len);
+	bname[len] = '\0';
 	return(bname);
 }
diff -ur libtar-1.2.11/compat/dirname.c libtar-1.2.11.modified/compat/dirname.c
--- libtar-1.2.11/compat/dirname.c	2000-10-27 18:16:34.000000000 +0200
+++ libtar-1.2.11.modified/compat/dirname.c	2012-01-18 23:06:20.000000000 +0100
@@ -34,17 +34,30 @@
 #include <errno.h>
 #include <string.h>
 #include <sys/param.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 char *
 openbsd_dirname(path)
 	const char *path;
 {
-	static char bname[MAXPATHLEN];
+	static char *bname = NULL;
+	static unsigned allocated = 0;
 	register const char *endp;
+	int len;
+
+	if (!allocated) {
+		allocated = 64;
+		bname = malloc(allocated);
+		if (!bname) {
+			allocated = 0;
+			return NULL;
+		}
+	}
 
 	/* Empty or NULL string gets treated as "." */
 	if (path == NULL || *path == '\0') {
-		(void)strcpy(bname, ".");
+		strcpy(bname, ".");
 		return(bname);
 	}
 
@@ -67,11 +80,18 @@
 		} while (endp > path && *endp == '/');
 	}
 
-	if (endp - path + 1 > sizeof(bname)) {
-		errno = ENAMETOOLONG;
-		return(NULL);
+	len = endp - path + 1;
+	if (len + 1 > allocated) {
+		unsigned new_allocated = 2*(len+1);
+		void *new_bname = malloc(new_allocated);
+		if (!new_bname)
+			return NULL;
+		allocated = new_allocated;
+		free(bname);
+		bname = new_bname;
 	}
-	(void)strncpy(bname, path, endp - path + 1);
-	bname[endp - path + 1] = '\0';
+
+	(void)strncpy(bname, path, len);
+	bname[len] = '\0';
 	return(bname);
 }
diff -ur libtar-1.2.11/lib/append.c libtar-1.2.11.modified/lib/append.c
--- libtar-1.2.11/lib/append.c	2003-01-07 02:40:59.000000000 +0100
+++ libtar-1.2.11.modified/lib/append.c	2012-01-20 14:28:57.000000000 +0100
@@ -17,6 +17,7 @@
 #include <fcntl.h>
 #include <sys/param.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 
 #ifdef STDC_HEADERS
 # include <stdlib.h>
@@ -38,7 +39,7 @@
 struct tar_ino
 {
 	ino_t ti_ino;
-	char ti_name[MAXPATHLEN];
+	char *ti_name;
 };
 typedef struct tar_ino tar_ino_t;
 
@@ -61,7 +62,7 @@
 	libtar_hashptr_t hp;
 	tar_dev_t *td = NULL;
 	tar_ino_t *ti = NULL;
-	char path[MAXPATHLEN];
+	char *path = NULL, *name = NULL;
 
 #ifdef DEBUG
 	printf("==> tar_append_file(TAR=0x%lx (\"%s\"), realname=\"%s\", "
@@ -135,25 +136,33 @@
 		if (ti == NULL)
 			return -1;
 		ti->ti_ino = s.st_ino;
-		snprintf(ti->ti_name, sizeof(ti->ti_name), "%s",
-			 savename ? savename : realname);
+		name = savename ? savename : realname; 
+		if ((ti->ti_name = malloc(strlen(name) + 1)) == NULL)
+			return -1;
+		snprintf(ti->ti_name, strlen(name) + 1, "%s", name);
 		libtar_hash_add(td->td_h, ti);
+		if (ti->ti_name != NULL)
+			free(ti->ti_name);
 	}
 
 	/* check if it's a symlink */
 	if (TH_ISSYM(t))
 	{
-		i = readlink(realname, path, sizeof(path));
+		if ((path = malloc(s.st_size + 1)) == NULL)
+			return -1;
+		i = readlink(realname, path, s.st_size);
 		if (i == -1)
+		{
+			free(path);
 			return -1;
-		if (i >= MAXPATHLEN)
-			i = MAXPATHLEN - 1;
+		}
 		path[i] = '\0';
 #ifdef DEBUG
 		printf("    tar_append_file(): encoding symlink \"%s\" -> "
 		       "\"%s\"...\n", realname, path);
 #endif
 		th_set_link(t, path);
+		free(path);
 	}
 
 	/* print file info */
diff -ur libtar-1.2.11/lib/decode.c libtar-1.2.11.modified/lib/decode.c
--- libtar-1.2.11/lib/decode.c	2012-01-14 17:16:35.000000000 +0100
+++ libtar-1.2.11.modified/lib/decode.c	2012-01-21 13:06:29.000000000 +0100
@@ -26,19 +26,26 @@
 char *
 th_get_pathname(TAR *t)
 {
-	static char filename[MAXPATHLEN];
+	static char *filename = NULL;
+	int len;
 
 	if (t->th_buf.gnu_longname)
 		return t->th_buf.gnu_longname;
 
 	if (t->th_buf.prefix[0] != '\0')
 	{
-		snprintf(filename, sizeof(filename), "%.155s/%.100s",
+		len = strlen(t->th_buf.prefix) + 1 + strlen(t->th_buf.name) + 1;
+		if ((filename = malloc(len + 1)) == NULL)
+			return NULL;
+		snprintf(filename, len + 1, "%.155s/%.100s",
 			 t->th_buf.prefix, t->th_buf.name);
 		return filename;
 	}
 
-	snprintf(filename, sizeof(filename), "%.100s", t->th_buf.name);
+	len = strlen(t->th_buf.name);
+	if ((filename = malloc(len + 1)) == NULL)
+		return NULL;
+	snprintf(filename, len + 1, "%.100s", t->th_buf.name);
 	return filename;
 }
 
diff -ur libtar-1.2.11/lib/extract.c libtar-1.2.11.modified/lib/extract.c
--- libtar-1.2.11/lib/extract.c	2003-03-03 00:58:07.000000000 +0100
+++ libtar-1.2.11.modified/lib/extract.c	2012-01-20 18:16:19.000000000 +0100
@@ -18,6 +18,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <utime.h>
+#include <string.h>
 
 #ifdef STDC_HEADERS
 # include <stdlib.h>
@@ -30,8 +31,8 @@
 
 struct linkname
 {
-	char ln_save[MAXPATHLEN];
-	char ln_real[MAXPATHLEN];
+	char *ln_save;
+	char *ln_real;
 };
 typedef struct linkname linkname_t;
 
@@ -97,8 +98,9 @@
 int
 tar_extract_file(TAR *t, char *realname)
 {
-	int i;
+	int i, len;
 	linkname_t *lnp;
+	char *pathname = NULL;
 
 	if (t->options & TAR_NOOVERWRITE)
 	{
@@ -140,15 +142,32 @@
 	lnp = (linkname_t *)calloc(1, sizeof(linkname_t));
 	if (lnp == NULL)
 		return -1;
-	strlcpy(lnp->ln_save, th_get_pathname(t), sizeof(lnp->ln_save));
-	strlcpy(lnp->ln_real, realname, sizeof(lnp->ln_real));
+	pathname = th_get_pathname(t);
+	len = strlen(pathname);
+	if ((lnp->ln_save = malloc(len + 1)) == NULL)
+		return -1;
+	strlcpy(lnp->ln_save, pathname, len + 1);
+	len = strlen(realname);
+	if ((lnp->ln_real = malloc(len + 1)) == NULL)
+	  {
+		free(lnp->ln_save);
+		return -1;
+	  }
+	strlcpy(lnp->ln_real, realname, len + 1);
 #ifdef DEBUG
 	printf("tar_extract_file(): calling libtar_hash_add(): key=\"%s\", "
-	       "value=\"%s\"\n", th_get_pathname(t), realname);
+	       "value=\"%s\"\n", pathname, realname);
 #endif
 	if (libtar_hash_add(t->h, lnp) != 0)
+	{
+		free(lnp->ln_save);
+		free(lnp->ln_real);
 		return -1;
+	}
 
+	/* FIXME: check if they can be freed here */
+	free(lnp->ln_save);
+	free(lnp->ln_real);
 	return 0;
 }
 
diff -ur libtar-1.2.11/lib/libtar.h libtar-1.2.11.modified/lib/libtar.h
--- libtar-1.2.11/lib/libtar.h	2003-01-07 02:40:59.000000000 +0100
+++ libtar-1.2.11.modified/lib/libtar.h	2012-01-17 17:49:39.000000000 +0100
@@ -260,7 +260,7 @@
 int ino_hash(ino_t *inode);
 
 /* create any necessary dirs */
-int mkdirhier(char *path);
+int mkdirhier(const char *path);
 
 /* calculate header checksum */
 int th_crc_calc(TAR *t);
diff -ur libtar-1.2.11/lib/util.c libtar-1.2.11.modified/lib/util.c
--- libtar-1.2.11/lib/util.c	2003-01-07 02:41:00.000000000 +0100
+++ libtar-1.2.11.modified/lib/util.c	2012-01-18 23:43:01.000000000 +0100
@@ -15,6 +15,7 @@
 #include <stdio.h>
 #include <sys/param.h>
 #include <errno.h>
+#include <stdlib.h>
 
 #ifdef STDC_HEADERS
 # include <string.h>
@@ -25,13 +26,15 @@
 int
 path_hashfunc(char *key, int numbuckets)
 {
-	char buf[MAXPATHLEN];
+	char *buf;
 	char *p;
+	int i;
 
-	strcpy(buf, key);
+	buf = strdup(key);
 	p = basename(buf);
-
-	return (((unsigned int)p[0]) % numbuckets);
+	i = ((unsigned int)p[0]) % numbuckets;
+	free(buf);
+	return (i);
 }
 
 
@@ -66,7 +69,6 @@
 	return *inode % 256;
 }
 
-
 /*
 ** mkdirhier() - create all directories in a given path
 ** returns:
@@ -75,17 +77,28 @@
 **	-1 (and sets errno)	error
 */
 int
-mkdirhier(char *path)
+mkdirhier(const char *path)
 {
-	char src[MAXPATHLEN], dst[MAXPATHLEN] = "";
-	char *dirp, *nextp = src;
-	int retval = 1;
+	char *src, *dst = NULL;
+	char *dirp, *nextp = NULL;
+	int retval = 1, len;
+
+	len = strlen(path);
+	if ((src = strdup(path)) == NULL)
+	{
+		errno = ENOMEM;
+		return -1;
+	}
+	nextp = src;
 
-	if (strlcpy(src, path, sizeof(src)) > sizeof(src))
+	/* Make room for // with absolute paths */
+	if ((dst = malloc(len + 2)) == NULL)
 	{
-		errno = ENAMETOOLONG;
+		free(src);
+		errno = ENOMEM;
 		return -1;
 	}
+	dst[0] = '\0';
 
 	if (path[0] == '/')
 		strcpy(dst, "/");
@@ -102,12 +115,18 @@
 		if (mkdir(dst, 0777) == -1)
 		{
 			if (errno != EEXIST)
+			{
+				free(src);
+				free(dst);
 				return -1;
+			}
 		}
 		else
 			retval = 0;
 	}
 
+	free(src);
+	free(dst);
 	return retval;
 }
 
diff -ur libtar-1.2.11/lib/wrapper.c libtar-1.2.11.modified/lib/wrapper.c
--- libtar-1.2.11/lib/wrapper.c	2003-01-07 02:41:00.000000000 +0100
+++ libtar-1.2.11.modified/lib/wrapper.c	2012-01-21 13:29:36.000000000 +0100
@@ -16,6 +16,7 @@
 #include <sys/param.h>
 #include <dirent.h>
 #include <errno.h>
+#include <stdlib.h>
 
 #ifdef STDC_HEADERS
 # include <string.h>
@@ -26,8 +27,8 @@
 tar_extract_glob(TAR *t, char *globname, char *prefix)
 {
 	char *filename;
-	char buf[MAXPATHLEN];
-	int i;
+	char *buf = NULL;
+	int i, len;
 
 	while ((i = th_read(t)) == 0)
 	{
@@ -41,13 +42,27 @@
 		if (t->options & TAR_VERBOSE)
 			th_print_long_ls(t);
 		if (prefix != NULL)
-			snprintf(buf, sizeof(buf), "%s/%s", prefix, filename);
+		{
+			len = strlen(prefix) + 1 + strlen(filename);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			snprintf(buf, len + 1, "%s/%s", prefix, filename);
+		}
 		else
-			strlcpy(buf, filename, sizeof(buf));
-		if (tar_extract_file(t, filename) != 0)
+		{
+			len = strlen(filename);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			strlcpy(buf, filename, len + 1);
+		}
+		if (tar_extract_file(t, buf) != 0)
+		{
+			free(buf);
 			return -1;
+		}
 	}
 
+	free(buf);
 	return (i == 1 ? 0 : -1);
 }
 
@@ -56,8 +71,8 @@
 tar_extract_all(TAR *t, char *prefix)
 {
 	char *filename;
-	char buf[MAXPATHLEN];
-	int i;
+	char *buf = NULL;
+	int i, len;
 
 #ifdef DEBUG
 	printf("==> tar_extract_all(TAR *t, \"%s\")\n",
@@ -69,21 +84,36 @@
 #ifdef DEBUG
 		puts("    tar_extract_all(): calling th_get_pathname()");
 #endif
+
 		filename = th_get_pathname(t);
 		if (t->options & TAR_VERBOSE)
 			th_print_long_ls(t);
 		if (prefix != NULL)
-			snprintf(buf, sizeof(buf), "%s/%s", prefix, filename);
+		{
+			len = strlen(prefix) + 1 + strlen(filename);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			snprintf(buf, len + 1, "%s/%s", prefix, filename);
+		}
 		else
-			strlcpy(buf, filename, sizeof(buf));
+		{
+			len = strlen(filename);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			strlcpy(buf, filename, len + 1);
+		}
 #ifdef DEBUG
 		printf("    tar_extract_all(): calling tar_extract_file(t, "
 		       "\"%s\")\n", buf);
 #endif
 		if (tar_extract_file(t, buf) != 0)
+		{
+			free(buf);
 			return -1;
+		}
 	}
 
+	free(buf);
 	return (i == 1 ? 0 : -1);
 }
 
@@ -91,11 +121,12 @@
 int
 tar_append_tree(TAR *t, char *realdir, char *savedir)
 {
-	char realpath[MAXPATHLEN];
-	char savepath[MAXPATHLEN];
+	char *realpath = NULL;
+	char *savepath = NULL;
 	struct dirent *dent;
 	DIR *dp;
 	struct stat s;
+	int len;
 
 #ifdef DEBUG
 	printf("==> tar_append_tree(0x%lx, \"%s\", \"%s\")\n",
@@ -122,11 +153,19 @@
 		    strcmp(dent->d_name, "..") == 0)
 			continue;
 
-		snprintf(realpath, MAXPATHLEN, "%s/%s", realdir,
+		len = strlen(realdir) + 1 + strlen(dent->d_name);
+		if ((realpath = malloc(len + 1)) == NULL)
+			return -1;
+		snprintf(realpath, len + 1, "%s/%s", realdir,
 			 dent->d_name);
 		if (savedir)
-			snprintf(savepath, MAXPATHLEN, "%s/%s", savedir,
+		{
+			len = strlen(savepath) + 1 + strlen(dent->d_name);
+			if ((realpath = malloc(len + 1)) == NULL)
+				return -1;
+			snprintf(savepath, len + 1, "%s/%s", savedir,
 				 dent->d_name);
+		}
 
 		if (lstat(realpath, &s) != 0)
 			return -1;
@@ -135,15 +174,25 @@
 		{
 			if (tar_append_tree(t, realpath,
 					    (savedir ? savepath : NULL)) != 0)
+			{
+				free(realpath);
+				free(savepath);
 				return -1;
+			}
 			continue;
 		}
 
 		if (tar_append_file(t, realpath,
 				    (savedir ? savepath : NULL)) != 0)
-			return -1;
+			{
+				free(realpath);
+				free(savepath);
+				return -1;
+			}
 	}
 
+	free(realpath);
+	free(savepath);
 	closedir(dp);
 
 	return 0;
diff -ur libtar-1.2.11/libtar/libtar.c libtar-1.2.11.modified/libtar/libtar.c
--- libtar-1.2.11/libtar/libtar.c	2012-01-14 17:16:35.000000000 +0100
+++ libtar-1.2.11.modified/libtar/libtar.c	2012-01-21 15:04:14.000000000 +0100
@@ -111,8 +111,9 @@
 {
 	TAR *t;
 	char *pathname;
-	char buf[MAXPATHLEN];
+	char *buf = NULL;
 	libtar_listptr_t lp;
+	int len;
 
 	if (tar_open(&t, tarfile,
 #ifdef HAVE_LIBZ
@@ -133,15 +134,26 @@
 	{
 		pathname = (char *)libtar_listptr_data(&lp);
 		if (pathname[0] != '/' && rootdir != NULL)
-			snprintf(buf, sizeof(buf), "%s/%s", rootdir, pathname);
+		{
+			len = strlen(rootdir) + 1 + strlen(pathname);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			snprintf(buf, len + 1, "%s/%s", rootdir, pathname);
+		}
 		else
-			strlcpy(buf, pathname, sizeof(buf));
+		{
+			len = strlen(pathname);
+			if ((buf = malloc(len + 1)) == NULL)
+				return -1;
+			strlcpy(buf, pathname, len + 1);
+		}
 		if (tar_append_tree(t, buf, pathname) != 0)
 		{
 			fprintf(stderr,
 				"tar_append_tree(\"%s\", \"%s\"): %s\n", buf,
 				pathname, strerror(errno));
 			tar_close(t);
+			free(buf);
 			return -1;
 		}
 	}
@@ -150,15 +162,18 @@
 	{
 		fprintf(stderr, "tar_append_eof(): %s\n", strerror(errno));
 		tar_close(t);
+		free(buf);
 		return -1;
 	}
 
 	if (tar_close(t) != 0)
 	{
 		fprintf(stderr, "tar_close(): %s\n", strerror(errno));
+		free(buf);
 		return -1;
 	}
 
+	free(buf);
 	return 0;
 }
 
@@ -286,7 +301,7 @@
 {
 	char *tarfile = NULL;
 	char *rootdir = NULL;
-	int c;
+	int c, i;
 	int mode = 0;
 	libtar_list_t *l;
 
@@ -348,15 +363,22 @@
 	switch (mode)
 	{
 	case MODE_EXTRACT:
-		return extract(argv[optind], rootdir);
+		i = extract(argv[optind], rootdir);
+		free(rootdir);
+		return i;
 	case MODE_CREATE:
 		tarfile = argv[optind];
 		l = libtar_list_new(LIST_QUEUE, NULL);
 		for (c = optind + 1; c < argc; c++)
 			libtar_list_add(l, argv[c]);
-		return create(tarfile, rootdir, l);
+		i = create(tarfile, rootdir, l);
+		free(l);
+		free(rootdir);
+		return i;
 	case MODE_LIST:
-		return list(argv[optind]);
+		free(rootdir);
+		i = list(argv[optind]);
+		return i;;
 	default:
 		break;
 	}

Reply to: