9front - general discussion about 9front
 help / color / mirror / Atom feed
From: ori@eigenstate.org
To: 9front@9front.org
Subject: tar bugfixes
Date: Sun, 3 Nov 2019 09:01:50 -0800	[thread overview]
Message-ID: <E866495E0A1D609B4C3E78610564E81B@eigenstate.org> (raw)

Our tar had 2 issues. First, it would crash on some systems when turning
absolute paths into relative paths, because not all code paths to produce
a name included an invisible 2 bytes of slack in the buffer. This removes
this hidden buffer space, and just allocates a new string.

Second, we would sometimes get confused when extracting tarballs with
long names. Tar specifies that a filename ending with '/' is a directory.
We would look at the short name, and if the short name ended with a '/',
we would decide it was a directory. With long names, this fails if the
100th character is a '/'. This switches to using the long name when
deciding the size for extraction, and trusts the header size when just
skipping forward in the stream.

Tested by Petter on IRC for the first crash, and by extracting the Go
tarball for the second issue.

I may remove the stack allocated buffers in a future commit.

If ok, I can commit.

diff -r fe8aefb6ab34 sys/src/cmd/tar.c
--- a/sys/src/cmd/tar.c	Mon Oct 28 14:12:44 2019 -0700
+++ b/sys/src/cmd/tar.c	Sun Nov 03 09:01:25 2019 -0800
@@ -496,7 +496,7 @@
 {
 	int pfxlen, namlen;
 	char *fullname;
-	static char fullnamebuf[2+Maxname+1];  /* 2+ for ./ on relative names */
+	static char fullnamebuf[Maxname+1];  /* 2+ for ./ on relative names */
 
 	fullname = fullnamebuf+2;
 	namlen = strnlen(hp->name, sizeof hp->name);
@@ -516,11 +516,11 @@
 }
 
 static int
-isdir(Hdr *hp)
+isdir(Hdr *hp, char *name)
 {
 	/* the mode test is ugly but sometimes necessary */
 	return hp->linkflag == LF_DIR ||
-		strrchr(name(hp), '\0')[-1] == '/' ||
+		strrchr(name, '\0')[-1] == '/' ||
 		(strtoul(hp->mode, nil, 8)&0170000) == 040000;
 }
 
@@ -605,9 +605,9 @@
  * return the number of bytes recorded in the archive.
  */
 static Off
-arsize(Hdr *hp)
+arsize(Hdr *hp, char *fname)
 {
-	if(isdir(hp) || islink(hp->linkflag))
+	if(isdir(hp, fname) || islink(hp->linkflag))
 		return 0;
 	return hdrsize(hp);
 }
@@ -651,7 +651,7 @@
 		} while (hdrcksum == -1 || chksum(hp) != hdrcksum);
 		fprint(2, "found %s\n", name(hp));
 	}
-	nexthdr += Tblock*(1 + BYTES2TBLKS(arsize(hp)));
+	nexthdr += Tblock*(1 + BYTES2TBLKS(hdrsize(hp)));
 	return hp;
 }
 
@@ -670,7 +670,7 @@
 	char *sl, *osl;
 	String *slname = nil;
 
-	if (isdir(hp)) {
+	if (isdir(hp, name)) {
 		slname = s_new();
 		s_append(slname, name);
 		s_append(slname, "/");		/* posix requires this */
@@ -898,7 +898,7 @@
 	if (usefile && !docreate) {
 		/* skip quickly to the end */
 		while ((hp = readhdr(ar)) != nil) {
-			bytes = arsize(hp);
+			bytes = hdrsize(hp);
 			for (blksleft = BYTES2TBLKS(bytes);
 			     blksleft > 0 && getblkrd(ar, Justnxthdr) != nil;
 			     blksleft -= blksread) {
@@ -1147,10 +1147,11 @@
 	long mtime = strtol(hp->mtime, nil, 8);
 	ulong mode = strtoul(hp->mode, nil, 8) & 0777;
 	Off bytes = hdrsize(hp);		/* for printing */
-	ulong blksleft = BYTES2TBLKS(arsize(hp));
+	ulong blksleft = BYTES2TBLKS(arsize(hp, fname));
+	char *path;
 
 	/* fiddle name, figure out mode and blocks */
-	if (isdir(hp)) {
+	if (isdir(hp, fname)) {
 		mode |= DMDIR|0700;
 		dir = 1;
 	}
@@ -1162,16 +1163,19 @@
 		blksleft = 0;
 		break;
 	}
-	if (relative)
-		if(fname[0] == '/')
-			*--fname = '.';
-		else if(fname[0] == '#'){
-			*--fname = '/';
-			*--fname = '.';
-		}
+
+	if ((path = malloc(2 + strlen(fname) + 1)) == nil)
+		sysfatal("alloc path: %r\n");
+	if(relative && fname[0] == '/')
+		strcpy(path, ".");
+	else if(relative && fname[0] == '#')
+		strcpy(path, "./");
+	else
+		path[0] = '\0';
+	strcat(path, fname);
 
 	if (verb == Xtract)
-		fd = openfname(hp, fname, dir, mode);
+		fd = openfname(hp, path, dir, mode);
 	else if (verbose) {
 		char *cp = ctime(mtime);
 
@@ -1192,6 +1196,7 @@
 			wrmeta(fd, hp, mtime, mode);
 		close(fd);
 	}
+	free(path);
 }
 
 static void
@@ -1200,7 +1205,7 @@
 	ulong blksleft, blksread;
 	Hdr *hbp;
 
-	for (blksleft = BYTES2TBLKS(arsize(hp)); blksleft > 0;
+	for (blksleft = BYTES2TBLKS(arsize(hp, fname)); blksleft > 0;
 	     blksleft -= blksread) {
 		hbp = getblkrd(ar, Justnxthdr);
 		if (hbp == nil)
@@ -1226,7 +1231,7 @@
 	fname = name(hp);
 	if(hp->linkflag == LF_LONGNAME){
 		p = namebuf;
-		for (blksleft = BYTES2TBLKS(arsize(hp)); blksleft > 0;
+		for (blksleft = BYTES2TBLKS(hdrsize(hp)); blksleft > 0;
 		     blksleft -= blksread) {
 			hp = getblkrd(ar, Alldata);
 			if (hp == nil)



                 reply	other threads:[~2019-11-03 17:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E866495E0A1D609B4C3E78610564E81B@eigenstate.org \
    --to=ori@eigenstate.org \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).