söndagen den 9 februari 2014 19.35.06 skrev Magnus Holmgren: > fredagen den 31 januari 2014 23.01.54 skrev du: > > I've attached an updated patch solving some of the issues upstream > > commented on in the last patch. I have not had time to update all of > > it yet, but thought it best to check if it would get accepted into > > Debian even if upstream refuse to include it. > > I think you've missed a couple of free()s, at least in the various loops in > wrapper.c. The final free()s could be moved inside the loops, but the plan > was to reuse the already allocated buffer if it's large enough, wasn't it? > > Using strlcpy after having already ascertained that the buffer is large > enough is unnecessary (the existing code used strlcpy, it's not something > you added, I know).
Also you seem to have messed upp savepath in tar_append_tree. I've updated your updated patch. Any comments? Perhaps my realloc_if_needed() code is overly complicated. I'm guessing that free()ing buf and malloc()ing it again is in practice going to reuse the same memory anyway, and the overhead is negligible. Since 1.2.21 isn't released yet I'm going to include the commit that did away with the static buffer and introduced th_pathname rather than creating an orig.tar.gz from the latest git commit, although that might not be a bad idea either. -- Magnus Holmgren [email protected] Debian Developer
Author: Svante Signell <[email protected]> Author: Petter Reinholdtsen <[email protected]> Author: Magnus Holmgren <[email protected]> Bug-Debian: http://bugs.debian.org/657116 Description: Fix FTBFS on Hurd by dynamically allocating path names. Depends on no_static_buffers.patch, which introduced the th_pathname field. --- a/compat/basename.c +++ b/compat/basename.c @@ -34,13 +34,25 @@ static char rcsid[] = "$OpenBSD: basenam #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; + static size_t allocated = 0; register const char *endp, *startp; + int len = 0; + + 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') { @@ -64,11 +76,19 @@ openbsd_basename(path) while (startp > path && *(startp - 1) != '/') startp--; - if (endp - startp + 1 > sizeof(bname)) { - errno = ENAMETOOLONG; - return(NULL); + len = endp - startp + 1; + + if (len + 1 > allocated) { + size_t 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, startp, endp - startp + 1); - bname[endp - startp + 1] = '\0'; + + (void)strncpy(bname, startp, len); + bname[len] = '\0'; return(bname); } --- a/compat/dirname.c +++ b/compat/dirname.c @@ -34,13 +34,25 @@ static char rcsid[] = "$OpenBSD: dirname #include <errno.h> #include <string.h> #include <sys/param.h> +#include <stdlib.h> char * openbsd_dirname(path) const char *path; { - static char bname[MAXPATHLEN]; + static char *bname = NULL; + static size_t 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') { --- a/lib/append.c +++ b/lib/append.c @@ -38,7 +38,7 @@ typedef struct tar_dev tar_dev_t; struct tar_ino { ino_t ti_ino; - char ti_name[MAXPATHLEN]; + char ti_name[]; }; typedef struct tar_ino tar_ino_t; @@ -61,7 +61,7 @@ tar_append_file(TAR *t, const char *real libtar_hashptr_t hp; tar_dev_t *td = NULL; tar_ino_t *ti = NULL; - char path[MAXPATHLEN]; + char *path = NULL; #ifdef DEBUG printf("==> tar_append_file(TAR=0x%lx (\"%s\"), realname=\"%s\", " @@ -126,34 +126,39 @@ tar_append_file(TAR *t, const char *real } else { + const char *name; #ifdef DEBUG printf("+++ adding entry: device (0x%lx,0x%lx), inode %ld " "(\"%s\")...\n", major(s.st_dev), minor(s.st_dev), s.st_ino, realname); #endif - ti = (tar_ino_t *)calloc(1, sizeof(tar_ino_t)); + name = savename ? savename : realname; + ti = (tar_ino_t *)calloc(1, sizeof(tar_ino_t) + strlen(name) + 1); if (ti == NULL) return -1; ti->ti_ino = s.st_ino; - snprintf(ti->ti_name, sizeof(ti->ti_name), "%s", - savename ? savename : realname); + snprintf(ti->ti_name, strlen(name) + 1, "%s", name); libtar_hash_add(td->td_h, ti); } /* 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 */ --- a/lib/decode.c +++ b/lib/decode.c @@ -29,10 +29,13 @@ th_get_pathname(TAR *t) if (t->th_buf.gnu_longname) return t->th_buf.gnu_longname; + size_t pathlen = + strlen(t->th_buf.prefix) + strlen(t->th_buf.name) + 2; + /* allocate the th_pathname buffer if not already */ if (t->th_pathname == NULL) { - t->th_pathname = malloc(MAXPATHLEN * sizeof(char)); + t->th_pathname = malloc(pathlen); if (t->th_pathname == NULL) /* out of memory */ return NULL; @@ -40,11 +43,11 @@ th_get_pathname(TAR *t) if (t->th_buf.prefix[0] == '\0') { - snprintf(t->th_pathname, MAXPATHLEN, "%.100s", t->th_buf.name); + snprintf(t->th_pathname, pathlen, "%.100s", t->th_buf.name); } else { - snprintf(t->th_pathname, MAXPATHLEN, "%.155s/%.100s", + snprintf(t->th_pathname, pathlen, "%.155s/%.100s", t->th_buf.prefix, t->th_buf.name); } --- a/lib/util.c +++ b/lib/util.c @@ -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); } @@ -77,15 +80,26 @@ ino_hash(ino_t *inode) int mkdirhier(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 +116,18 @@ mkdirhier(char *path) if (mkdir(dst, 0777) == -1) { if (errno != EEXIST) + { + free(src); + free(dst); return -1; + } } else retval = 0; } + free(src); + free(dst); return retval; } --- a/lib/wrapper.c +++ b/lib/wrapper.c @@ -16,18 +16,34 @@ #include <sys/param.h> #include <dirent.h> #include <errno.h> +#include <stdlib.h> #ifdef STDC_HEADERS # include <string.h> #endif +inline void * +realloc_if_needed(void *buf, size_t *cursize, size_t newsize) +{ + void *tmp; + if (newsize > *cursize || !buf) + { + if ((tmp = realloc(buf, newsize)) == NULL) + { + free(buf); + return NULL; + } + } + return buf; +} int tar_extract_glob(TAR *t, char *globname, char *prefix) { char *filename; - char buf[MAXPATHLEN]; - int i; + char *buf = NULL; + size_t bufsize = 0; + int i, len; while ((i = th_read(t)) == 0) { @@ -41,13 +57,27 @@ tar_extract_glob(TAR *t, char *globname, 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 = realloc_if_needed(buf, &bufsize, len + 1)) == NULL) + return -1; + sprintf(buf, "%s/%s", prefix, filename); + } else - strlcpy(buf, filename, sizeof(buf)); + { + len = strlen(filename); + if ((buf = realloc_if_needed(buf, &bufsize, len + 1)) == NULL) + return -1; + strcpy(buf, filename); + } if (tar_extract_file(t, buf) != 0) + { + free(buf); return -1; + } } + free(buf); return (i == 1 ? 0 : -1); } @@ -56,8 +86,9 @@ int tar_extract_all(TAR *t, char *prefix) { char *filename; - char buf[MAXPATHLEN]; - int i; + char *buf = NULL; + size_t bufsize = 0; + int i, len; #ifdef DEBUG printf("==> tar_extract_all(TAR *t, \"%s\")\n", @@ -69,21 +100,36 @@ tar_extract_all(TAR *t, char *prefix) #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 = realloc_if_needed(buf, &bufsize, len + 1)) == NULL) + return -1; + sprintf(buf, "%s/%s", prefix, filename); + } else - strlcpy(buf, filename, sizeof(buf)); + { + len = strlen(filename); + if ((buf = realloc_if_needed(buf, &bufsize, len + 1)) == NULL) + return -1; + strcpy(buf, filename); + } #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 +137,14 @@ tar_extract_all(TAR *t, char *prefix) int tar_append_tree(TAR *t, char *realdir, char *savedir) { - char realpath[MAXPATHLEN]; - char savepath[MAXPATHLEN]; + char *realpath = NULL; + size_t realpathsize = 0; + char *savepath = NULL; + size_t savepathsize = 0; struct dirent *dent; DIR *dp; struct stat s; + int len; #ifdef DEBUG printf("==> tar_append_tree(0x%lx, \"%s\", \"%s\")\n", @@ -122,11 +171,19 @@ tar_append_tree(TAR *t, char *realdir, c strcmp(dent->d_name, "..") == 0) continue; - snprintf(realpath, MAXPATHLEN, "%s/%s", realdir, + len = strlen(realdir) + 1 + strlen(dent->d_name); + if ((realpath = realloc_if_needed(realpath, &realpathsize, 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(savedir) + 1 + strlen(dent->d_name); + if ((savepath = realloc_if_needed(savepath, &savepathsize, len + 1)) == NULL) + return -1; + snprintf(realpath, len + 1, "%s/%s", savedir, dent->d_name); + } if (lstat(realpath, &s) != 0) return -1; @@ -135,15 +192,25 @@ tar_append_tree(TAR *t, char *realdir, c { 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) + { + free(realpath); + free(savepath); return -1; + } } + free(realpath); + free(savepath); closedir(dp); return 0; --- a/libtar/libtar.c +++ b/libtar/libtar.c @@ -111,8 +111,9 @@ create(char *tarfile, char *rootdir, lib { 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,17 +134,29 @@ create(char *tarfile, char *rootdir, lib { 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; } + free(buf); } if (tar_append_eof(t) != 0)
signature.asc
Description: This is a digitally signed message part.
