Subject: Re: problems untarring 20020526-1.6A
To: Tim Goodwin <tjg@star.le.ac.uk>
From: Luke Mewburn <lukem@wasabisystems.com>
List: netbsd-bugs
Date: 06/28/2002 02:31:50
On Thu, Jun 27, 2002 at 03:21:50PM -0000, Tim Goodwin wrote:
  | > did anyone take a look on what's actually causing this problem? (is it
  | > nbpax? if yes, why?  do someone know how to fix it and eventuelly upload
  | > a new working snapshot?)
  | 
  | Yes, I've just found the bug in `pax -M'.
  | 
  | This is not sparc64 specific, but it does involve heap corruption, so
  | there's a good chance it won't show up on a different architecture.
  | 
  | 1. The problem: buf_subs.c::bufpt is being corrupted; this results in
  |    pax failing to write a complete header, and the broken releases
  |    that we've all seen.
  | 
  | 2. The cause: bufpt is being corrupted by code around line 550 of
  |    ftree.c (which is all inside a giant `if (Mflag)'; the problem only
  |    arises with `pax -M').  When fed with this input:
  | 
  |         . type=dir optional
  |         ./netbsd type=file
  | 
  |    the following code sets curdirlen to -1 (it is initially 1 for ".",
  |    but at this point ftnode->name is also ".").  Assigning to
  |    curdir[-1] stomps on bufpt.
  | 
  |    549                      if (ftnode != NULL) {
  |    550                              curdirlen -= strlen(ftnode->name) + 1;
  |    551                              curdir[curdirlen] = '\0';
  |    552                      }

good detective work!


  | 3. The fix: I don't understand the code 100%, but the bug seems to be
  |    that curdir should be updated based on the old ftnode->name (we've
  |    just finished dealing with this one, so need to strip its name out
  |    of curdir); the current, buggy code updates ftnode first, then
  |    updates curdir using the new ftnode.  The appended patch changes
  |    the order of these.  In trivial testing it works (warning: I
  |    haven't done extensive testing).  At the very least, it should be
  |    sufficient to make working releases!

could you try my patch instead?  


Index: ftree.c
===================================================================
RCS file: /cvsroot/basesrc/bin/pax/ftree.c,v
retrieving revision 1.20
diff -p -u -r1.20 ftree.c
--- ftree.c	2002/04/20 23:36:48	1.20
+++ ftree.c	2002/06/27 16:30:27
@@ -542,11 +542,10 @@ next_file(ARCHD *arcn)
 					set_ftime(ftent->fts_path,
 					    mtime, atime, 1);
 				}
+				ftnode = ftnode->parent;
 				if (ftnode->parent == ftnode)
 					ftnode = NULL;
-				else
-					ftnode = ftnode->parent;
-				if (ftnode != NULL) {
+				else {
 					curdirlen -= strlen(ftnode->name) + 1;
 					curdir[curdirlen] = '\0';
 				}

-- 
Luke Mewburn  <lukem@wasabisystems.com>  http://www.wasabisystems.com
Luke Mewburn     <lukem@netbsd.org>      http://www.netbsd.org
Wasabi Systems - NetBSD hackers for hire
NetBSD - the world's most portable UNIX-like operating system