Next round of work on ISO9660 support:

  * Reference-count the directory data so that
    we don't leak memory.
  * Correctly step through the directory records
    (skipping unrecognized extensions)
  * Use better defaults for file modes
  * Sort directory entries by offset of the end of the file
    rather than the beginning of the file.  This fixes a
    lot of "out-of-order" problems with zero-length files,
    in particular.
  * Style fixes, remove some debug code, add some error messages.

1 files changed, 151 insertions, 79 deletions

diff --git a/lib/libarchive/archive_read_support_format_iso9660.c b/lib/libarchive/archive_read_support_format_iso9660.c
index 99be38d..fc7d1ee 100644
--- a/lib/libarchive/archive_read_support_format_iso9660.c
+++ b/lib/libarchive/archive_read_support_format_iso9660.c
@@ -128,10 +128,12 @@ struct directory_record {
 /* In-memory storage for a directory record. */
 struct dir_rec {
 	struct dir_rec	*parent;
+	int		 refcount;
 	unsigned char	 flags;
 	uint64_t	 offset;  /* Offset on disk. */
 	uint64_t	 size;	/* File size in bytes. */
 	time_t		 mtime;	/* File last modified time. */
+	mode_t		 mode;
 	char		 name[1]; /* Null-terminated filename. */
 };
 
@@ -142,6 +144,10 @@ struct iso9660 {
 	int	bid; /* If non-zero, return this as our bid. */
 	struct archive_string pathname;
 
+	uint64_t	previous_offset;
+	uint64_t	previous_size;
+	struct archive_string previous_pathname;
+
 	/* TODO: Make this a heap for fast inserts and deletions. */
 	struct dir_rec **pending_files;
 	int	pending_files_allocated;
@@ -150,8 +156,7 @@ struct iso9660 {
 	uint64_t current_position;
 	ssize_t	logical_block_size;
 
-	off_t	entry_offset;
-	ssize_t	entry_padding;
+	off_t	entry_sparse_offset;
 	ssize_t	entry_bytes_remaining;
 };
 
@@ -162,9 +167,11 @@ static int	archive_read_format_iso9660_read_data(struct archive *,
 static int	archive_read_format_iso9660_read_header(struct archive *,
 		    struct archive_entry *);
 static const char *build_pathname(struct archive_string *, struct dir_rec *);
+static void	dump_isodirent(FILE *, const struct directory_record *);
 static time_t	isodate(const void *);
 static int	isPVD(struct iso9660 *, const char *);
 static struct dir_rec *next_entry(struct iso9660 *);
+static void	release_dirrec(struct iso9660 *, struct dir_rec *);
 static int	store_pending(struct iso9660 *, struct dir_rec *parent,
 		    const struct directory_record *);
 static int	toi(const void *p, int n);
@@ -227,6 +234,8 @@ archive_read_format_iso9660_bid(struct archive *a)
 		iso9660->bid = isPVD(iso9660, p);
 		if (iso9660->bid > 0)
 			return (iso9660->bid);
+		if (*p == '\xff') /* End-of-volume-descriptor marker. */
+			break;
 	}
 
 	/* We didn't find a valid PVD; return a bid of zero. */
@@ -270,69 +279,88 @@ archive_read_format_iso9660_read_header(struct archive *a,
 	dirrec = next_entry(iso9660);
 	if (dirrec == NULL)
 		return (ARCHIVE_EOF);
-	while (dirrec->offset < iso9660->current_position) {
-		archive_string_empty(&iso9660->pathname);
-		fprintf(stderr, "\n ** Ignoring out-of-order file `%s' (offset 0x%0x/size %d) current offset 0x%x\n", build_pathname(&iso9660->pathname, dirrec), (unsigned int)dirrec->offset, (int)dirrec->size, (unsigned int)iso9660->current_position);
-		dirrec = next_entry(iso9660);
-		if (dirrec == NULL)
-			return (ARCHIVE_EOF);
+
+	iso9660->entry_bytes_remaining = dirrec->size;
+	iso9660->entry_sparse_offset = 0; /* Offset for sparse-file-aware clients. */
+
+	/* Set up the entry structure with information about this entry. */
+	memset(&st, 0, sizeof(st));
+	st.st_mode = dirrec->mode;
+	st.st_mtime = dirrec->mtime;
+	st.st_size = iso9660->entry_bytes_remaining;
+	archive_entry_copy_stat(entry, &st);
+	archive_string_empty(&iso9660->pathname);
+	archive_entry_set_pathname(entry,
+	    build_pathname(&iso9660->pathname, dirrec));
+
+	/* If this entry points to the same data as the previous
+	 * entry, convert this into a hardlink to that entry.
+	 * But don't bother for zero-length files. */
+	if (dirrec->offset == iso9660->previous_offset
+	    && dirrec->size == iso9660->previous_size
+	    && dirrec->size > 0) {
+		archive_entry_set_hardlink(entry,
+		    iso9660->previous_pathname.s);
+		iso9660->entry_bytes_remaining = 0;
+		iso9660->entry_sparse_offset = 0;
+		release_dirrec(iso9660, dirrec);
+		return (ARCHIVE_OK);
+	}
+
+	/* If the offset is before our current position, we can't
+	 * seek backwards to extract it, so issue a warning. */
+	if (dirrec->offset < iso9660->current_position) {
+		archive_set_error(a, ARCHIVE_ERRNO_MISC,
+		    "Ignoring out-of-order file");
+		iso9660->entry_bytes_remaining = 0;
+		iso9660->entry_sparse_offset = 0;
+		release_dirrec(iso9660, dirrec);
+		return (ARCHIVE_WARN);
 	}
 
-	/* Seek forward to the start of that entry. */
+	/* Seek forward to the start of the entry. */
 	while (iso9660->current_position < dirrec->offset) {
 		ssize_t step = dirrec->offset - iso9660->current_position;
 		if (step > iso9660->logical_block_size)
 			step = iso9660->logical_block_size;
 		bytes_read = (a->compression_read_ahead)(a, &buff, step);
-		if (bytes_read <= 0)
+		if (bytes_read <= 0) {
+			release_dirrec(iso9660, dirrec);
 			return (ARCHIVE_FATAL);
+		}
 		if (bytes_read > step)
 			bytes_read = step;
 		iso9660->current_position += bytes_read;
 		(a->compression_read_consume)(a, bytes_read);
 	}
 
-	/* Set up the entry structure with information about this entry. */
-	memset(&st, 0, sizeof(st));
-	iso9660->entry_bytes_remaining = dirrec->size;
-	/* The following assumes the logical block size is a power of 2. */
-	iso9660->entry_padding = (iso9660->logical_block_size - 1 )
-	    & (-iso9660->entry_bytes_remaining);
-	iso9660->entry_offset = 0;
-	st.st_mode = 0444;
-	if (dirrec->flags & 0x02)
-		st.st_mode |= S_IFDIR;
-	else
-		st.st_mode |= S_IFREG;
-	st.st_mtime = dirrec->mtime;
-	st.st_size = iso9660->entry_bytes_remaining;
-	archive_entry_copy_stat(entry, &st);
-	archive_string_empty(&iso9660->pathname);
-	archive_entry_set_pathname(entry,
-	    build_pathname(&iso9660->pathname, dirrec));
+	iso9660->previous_size = dirrec->size;
+	iso9660->previous_offset = dirrec->offset;
+	archive_strcpy(&iso9660->previous_pathname, iso9660->pathname.s);
 
 	/* If this is a directory, read in all of the entries right now. */
 	if (S_ISDIR(st.st_mode)) {
 		while(iso9660->entry_bytes_remaining > 0) {
 			const void *block;
-			const char *p;
+			const unsigned char *p;
 			ssize_t step = iso9660->logical_block_size;
 			if (step > iso9660->entry_bytes_remaining)
 				step = iso9660->entry_bytes_remaining;
 			bytes_read = (a->compression_read_ahead)(a, &block, step);
 			if (bytes_read < step) {
-				archive_set_error(a, -1,
+				archive_set_error(a, ARCHIVE_ERRNO_MISC,
 	    "Failed to read full block when scanning ISO9660 directory list");
+				release_dirrec(iso9660, dirrec);
 				return (ARCHIVE_FATAL);
 			}
 			if (bytes_read > step)
 				bytes_read = step;
-			if (bytes_read > iso9660->entry_bytes_remaining)
-				bytes_read = iso9660->entry_bytes_remaining;
 			(a->compression_read_consume)(a, bytes_read);
 			iso9660->current_position += bytes_read;
 			iso9660->entry_bytes_remaining -= bytes_read;
-			for (p = block; *p != 0; p += *p) {
+			for (p = block;
+			     *p != 0 && p < (const unsigned char *)block + bytes_read;
+			     p += *p) {
 				const struct directory_record *dr
 				    = (const struct directory_record *)p;
 				/* Skip '.' entry. */
@@ -348,6 +376,7 @@ archive_read_format_iso9660_read_header(struct archive *a,
 		}
 	}
 
+	release_dirrec(iso9660, dirrec);
 	return (ARCHIVE_OK);
 }
 
@@ -359,35 +388,25 @@ archive_read_format_iso9660_read_data(struct archive *a,
 	struct iso9660 *iso9660;
 
 	iso9660 = *(a->pformat_data);
-	if (iso9660->entry_bytes_remaining > 0) {
-		bytes_read = (a->compression_read_ahead)(a, buff, 1);
-		if (bytes_read <= 0)
-			return (ARCHIVE_FATAL);
-		if (bytes_read > iso9660->entry_bytes_remaining)
-			bytes_read = iso9660->entry_bytes_remaining;
-		*size = bytes_read;
-		*offset = iso9660->entry_offset;
-		iso9660->entry_offset += bytes_read;
-		iso9660->entry_bytes_remaining -= bytes_read;
-		iso9660->current_position += bytes_read;
-		(a->compression_read_consume)(a, bytes_read);
-		return (ARCHIVE_OK);
-	} else {
-		while (iso9660->entry_padding > 0) {
-			bytes_read = (a->compression_read_ahead)(a, buff, 1);
-			if (bytes_read <= 0)
-				return (ARCHIVE_FATAL);
-			if (bytes_read > iso9660->entry_padding)
-				bytes_read = iso9660->entry_padding;
-			iso9660->current_position += bytes_read;
-			(a->compression_read_consume)(a, bytes_read);
-			iso9660->entry_padding -= bytes_read;
-		}
+	if (iso9660->entry_bytes_remaining <= 0) {
 		*buff = NULL;
 		*size = 0;
-		*offset = iso9660->entry_offset;
+		*offset = iso9660->entry_sparse_offset;
 		return (ARCHIVE_EOF);
 	}
+
+	bytes_read = (a->compression_read_ahead)(a, buff, 1);
+	if (bytes_read <= 0)
+		return (ARCHIVE_FATAL);
+	if (bytes_read > iso9660->entry_bytes_remaining)
+		bytes_read = iso9660->entry_bytes_remaining;
+	*size = bytes_read;
+	*offset = iso9660->entry_sparse_offset;
+	iso9660->entry_sparse_offset += bytes_read;
+	iso9660->entry_bytes_remaining -= bytes_read;
+	iso9660->current_position += bytes_read;
+	(a->compression_read_consume)(a, bytes_read);
+	return (ARCHIVE_OK);
 }
 
 static int
@@ -424,6 +443,9 @@ store_pending(struct iso9660 *iso9660, struct dir_rec *parent,
 
 	new_dirent = malloc(sizeof(*new_dirent) + isodirent->name_len[0] + 1);
 	new_dirent->parent = parent;
+	if (parent != NULL)
+		parent->refcount++;
+	new_dirent->refcount = 0;
 	new_dirent->flags = isodirent->flags[0];
 	new_dirent->offset = toi(isodirent->extent, 4)
 	    * iso9660->logical_block_size;
@@ -432,45 +454,80 @@ store_pending(struct iso9660 *iso9660, struct dir_rec *parent,
 	memcpy(new_dirent->name, isodirent->name, isodirent->name_len[0]);
 	new_dirent->name[(int)isodirent->name_len[0]] = '\0';
 
-	iso9660->pending_files[iso9660->pending_files_used++] = new_dirent;
+	if (isodirent->flags[0] & 0x02)
+		new_dirent->mode = S_IFDIR | 0700;
+	else
+		new_dirent->mode = S_IFREG | 0400;
 
+	iso9660->pending_files[iso9660->pending_files_used++] = new_dirent;
 
-	/* DEBUGGING */
 
+	/* DEBUGGING: Warn about attributes I don't yet fully support. */
 	if ((isodirent->flags[0] & ~0x02) != 0) {
-		archive_string_empty(&iso9660->pathname);
-		fprintf(stderr, "\n ** Unrecognized flag 0x%x in file %s\n",
-		    new_dirent->flags,
-		    build_pathname(&iso9660->pathname, new_dirent));
-	}
-
-	if (toi(isodirent->volume_sequence_number,2) != 1
-	    || isodirent->file_unit_size[0] != 0
-	    || isodirent->interleave[0] != 0
-	    || isodirent->ext_attr_length[0] != 0) {
-		fprintf(stderr, "\n ** Unhandled ISO9660 directory attribute:");
-		fprintf(stderr, " l %d, a %d, ext 0x%x, s %d, f 0x%02x, unt %d, ilv %d, seq %d: `%.*s' (%d)\n", isodirent->length[0], isodirent->ext_attr_length[0], toi(isodirent->extent, 4) * 2048, toi(isodirent->size, 4), isodirent->flags[0], isodirent->file_unit_size[0], isodirent->interleave[0], toi(isodirent->volume_sequence_number,2), isodirent->name_len[0], isodirent->name, isodirent->name_len[0]);
+		fprintf(stderr, "\n ** Unrecognized flag: ");
+		dump_isodirent(stderr, isodirent);
+		fprintf(stderr, "\n");
+	} else if (toi(isodirent->volume_sequence_number, 2) != 1) {
+		fprintf(stderr, "\n ** Unrecognized sequence number: ");
+		dump_isodirent(stderr, isodirent);
+		fprintf(stderr, "\n");
+	} else if (isodirent->file_unit_size[0] != 0) {
+		fprintf(stderr, "\n ** Unexpected file unit size: ");
+		dump_isodirent(stderr, isodirent);
+		fprintf(stderr, "\n");
+	} else if (isodirent->interleave[0] != 0) {
+		fprintf(stderr, "\n ** Unexpected interleave: ");
+		dump_isodirent(stderr, isodirent);
+		fprintf(stderr, "\n");
+	} else if (isodirent->ext_attr_length[0] != 0) {
+		fprintf(stderr, "\n ** Unexpected extended attribute length: ");
+		dump_isodirent(stderr, isodirent);
+		fprintf(stderr, "\n");
 	}
 
-
 	return (ARCHIVE_OK);
 }
 
+static void
+release_dirrec(struct iso9660 *iso9660, struct dir_rec *dr)
+{
+	struct dir_rec *parent;
+
+	if (dr->refcount == 0) {
+		dr->flags = 0xff;
+		parent = dr->parent;
+		free(dr);
+		if (parent != NULL) {
+			parent->refcount--;
+			release_dirrec(iso9660, parent);
+		}
+	}
+}
+
 static struct dir_rec *
 next_entry(struct iso9660 *iso9660)
 {
-	int least_index = 0;
-	uint64_t least_offset = iso9660->pending_files[0]->offset;
+	int least_index;
+	uint64_t least_end_offset;
 	int i;
 	struct dir_rec *r;
 
 	if (iso9660->pending_files_used < 1)
 		return (NULL);
 
+	/* Assume the first file in the list is the earliest on disk. */
+	least_index = 0;
+	least_end_offset = iso9660->pending_files[0]->offset
+	    + iso9660->pending_files[0]->size;
+
+	/* Now, try to find an earlier one. */
 	for(i = 0; i < iso9660->pending_files_used; i++) {
-		if (iso9660->pending_files[i]->offset < least_offset) {
-			least_offset = iso9660->pending_files[i]->offset;
+		/* Use the position of the file *end* as our comparison. */
+		uint64_t end_offset = iso9660->pending_files[i]->offset
+		    + iso9660->pending_files[i]->size;
+		if (least_end_offset > end_offset) {
 			least_index = i;
+			least_end_offset = end_offset;
 		}
 	}
 	r = iso9660->pending_files[least_index];
@@ -504,8 +561,8 @@ isodate(const void *p)
 	tm.tm_sec = v[5];
 	/* v[6] is the timezone offset, in 1/4-hour increments. */
 	offset = ((const signed char *)p)[6];
-	tm.tm_hour += offset / 4;
-	tm.tm_min += (offset % 4) * 15;
+	tm.tm_hour -= offset / 4;
+	tm.tm_min -= (offset % 4) * 15;
 	return (timegm(&tm));
 }
 
@@ -519,3 +576,18 @@ build_pathname(struct archive_string *as, struct dir_rec *dr)
 	archive_strcat(as, dr->name);
 	return (as->s);
 }
+
+static void
+dump_isodirent(FILE *out, const struct directory_record *isodirent)
+{
+	fprintf(out, " l %d,", isodirent->length[0]);
+	fprintf(out, " a %d,", isodirent->ext_attr_length[0]);
+	fprintf(out, " ext 0x%x,", toi(isodirent->extent, 4));
+	fprintf(out, " s %d,", toi(isodirent->size, 4));
+	fprintf(out, " f 0x%02x,", isodirent->flags[0]);
+	fprintf(out, " u %d,", isodirent->file_unit_size[0]);
+	fprintf(out, " ilv %d,", isodirent->interleave[0]);
+	fprintf(out, " seq %d,", toi(isodirent->volume_sequence_number,2));
+	fprintf(out, " nl %d:", isodirent->name_len[0]);
+	fprintf(out, " `%.*s'", isodirent->name_len[0], isodirent->name);
+}