>Number:         17412
>Category:       bin
>Synopsis:       pax -M builds broken archives
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 27 09:18:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Tim Goodwin
>Release:        NetBSD 1.6A
>Organization:
University of Leicester
	
>Environment:
Sun Ultra1, NetBSD 1.6A
	
System: NetBSD neon 1.6A NetBSD 1.6A (NEON) #0: Thu Jun 13 14:23:17 BST 2002 tjg@neon:/hck/src.de/sys/arch/sparc64/compile/NEON sparc64


>Description:
`pax -M', which is used in building releases, produces broken archive files.
	
>How-To-Repeat:
    $ cat > /tmp/mt
    . type=dir optional
    ./netbsd type=file
    $ pax -w -M -f /tmp/broken < /tmp/mt
    $ pax < /tmp/broken
    pax: cannot identify format
    ...

Or just `pax < release/binary/sets/kern-GENERIC.tgz'.
>Fix:
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                      }

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!


Tim.
Index: ftree.c
===================================================================
RCS file: /cvsroot/basesrc/bin/pax/ftree.c,v
retrieving revision 1.20
diff -u -r1.20 ftree.c
--- ftree.c	20 Apr 2002 23:36:48 -0000	1.20
+++ ftree.c	27 Jun 2002 15:11:25 -0000
@@ -544,11 +544,10 @@
 				}
 				if (ftnode->parent == ftnode)
 					ftnode = NULL;
-				else
-					ftnode = ftnode->parent;
-				if (ftnode != NULL) {
+				else {
 					curdirlen -= strlen(ftnode->name) + 1;
 					curdir[curdirlen] = '\0';
+					ftnode = ftnode->parent;
 				}
 			}
 		} while (ftnode != NULL && ftnode->flags & F_VISIT);
>Release-Note:
>Audit-Trail:
>Unformatted: