>Number:         11188
>Category:       bin
>Synopsis:       restore exceeds array bounds on dump of large filesystem
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Oct 10 08:57:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Olaf Seibert
>Release:        1.4.2
>Organization:
>Environment:
System: NetBSD klei.intern.polderland.nl 1.4.2 NetBSD 1.4.2 (KIELDRECHT) #11: Wed Jul 19 14:00:31 CEST 2000 root@kieldrecht:/usr/src/sys/arch/i386/compile/KIELDRECHT i386


>Description:
	Copying description of OpenBSD pr system/1398, reported on
	Mon Sep 11 15:40:02 MDT 2000 by jeff@kestrel.edu:

A backup created by 'dump' begins with TS_CLRI and TS_BITS maps.  The
method 'restore' uses to read these maps from a backup file is faulty if
the map is longer than 512 TP_BSIZE blocks.  With TP_BSIZE defined to
1024, and 8 bits per byte, and one bit per inode, that translate to a
potential problem if the file system has more than 4,194,304 inodes.
 
Here is one way the problem shows itself:
 
> /sbin/restore ivdf /tmp/backup-data
Verify tape and initialize maps
Tape block size is 32
Volume header (new inode format) 
Dump   date: Thu Sep  7 15:58:40 2000
Dumped from: the epoch
Level 0 dump of /export on quetzal:/dev/raid0c
Label: none
maxino = 7503873
hole in map
abort? [yn] Segmentation fault (core dumped)
 
 
Restore has a function 'getfile' that reads file entries from the
backup file.  A file can have holes in it, so header blocks in the
backup have an array c_addr[512] that is used to mark which blocks are
holes and which have valid data.  Restore uses this same function to
read out the map data.  When reading map data, the first thing restore
does is make sure the c_addr[] array in the TS_CLRI and TS_BITS
headers have all blocks marked as valid.  It does this by incrementing
each location in the array.  When there are more than 512 blocks,
restore will continue past the end of the array incrementing each
byte.  There are actually 860 bytes between the start of the c_addr[]
array and the end of the TS_CLRI or TS_BITS header, but if any of
these bytes, after incrementing, get a value of zero, then the getfile
function will treat the corresponding block as a hole, and you get the
error shown above.  If more than 860 blocks, then restore will
increment locations beyond the header block itself, and possibly fail
in more mysterious ways.

>How-To-Repeat:
If the target filesystem is large enough:
dump 0f - / | restore ivdf -

In my case, my backup had 916 blocks of map data, so I exceeded the
limits of the header block.

>Fix:
The patch below removes the use of c_addr[] for TS_CLRI and TS_BITS
headers.  In the 'getfile' routine I've added a specific test for
TS_CLRI and TS_BITS data, and if so, it does not use c_addr[] and
instead unconditionally treats each block as valid.

[ patch adjusted for NetBSD 1.4.2; only the line numbers differ: ]

--- tape.c.orig	Tue Oct 10 16:57:41 2000
+++ tape.c	Tue Oct 10 17:05:04 2000
@@ -701,7 +701,7 @@
 	gettingfile++;
 loop:
 	for (i = 0; i < spcl.c_count; i++) {
-		if (spcl.c_addr[i]) {
+		if ((spcl.c_type == TS_CLRI) || (spcl.c_type == TS_BITS) || (spcl.c_addr[i])) {
 			readtape(&buf[curblk++][0]);
 			if (curblk == fssize / TP_BSIZE) {
 				(*fill)((char *)buf, (long)(size > TP_BSIZE ?
@@ -719,9 +719,12 @@
 				TP_BSIZE : size));
 		}
 		if ((size -= TP_BSIZE) <= 0) {
-			for (i++; i < spcl.c_count; i++)
+			for (i++; i < spcl.c_count; i++) {
+				if ((spcl.c_type == TS_CLRI) || (spcl.c_type == TS_BITS))
+					panic("excess tape blocks after map data\n");
 				if (spcl.c_addr[i])
 					readtape(junk);
+			}
 			break;
 		}
 	}
@@ -1126,8 +1129,6 @@
 		 */
 		buf->c_inumber = 0;
 		buf->c_dinode.di_size = buf->c_count * TP_BSIZE;
-		for (i = 0; i < buf->c_count; i++)
-			buf->c_addr[i]++;
 		break;
 
 	case TS_TAPE:
@@ -1206,7 +1207,10 @@
 	fprintf(stderr, "\n");
 newcalc:
 	blks = 0;
-	if (header->c_type != TS_END)
+
+	if ((header->c_type == TS_CLRI) || (header->c_type == TS_BITS))
+		blks = header->c_count;
+	else if (header->c_type != TS_END)
 		for (i = 0; i < header->c_count; i++)
 			if (header->c_addr[i] != 0)
 				blks++;

>Release-Note:
>Audit-Trail:
>Unformatted:

-Olaf.
-- 
___ Olaf 'Rhialto' Seibert - rhialto@polder    -- Ah only did well at school
\X/ land.nl       -- tae git intae an O level class tae git away fae Begbie.
Hi! I am a .signature virus. Copy me into your .signature to help me spread.