From: Eric Biggers <ebiggers3@gmail.com>
Date: Wed, 21 Oct 2015 03:26:23 +0000 (-0500)
Subject: Improved reporting of concurrent modifications and corruptions
X-Git-Tag: v1.8.3~32
X-Git-Url: https://wimlib.net/git/?p=wimlib;a=commitdiff_plain;h=6a9854db6d6ff8f5dd3f28a82946dd1ed673a36a

Improved reporting of concurrent modifications and corruptions
---

diff --git a/include/wimlib.h b/include/wimlib.h
index 6842e35d..f68cb2b8 100644
--- a/include/wimlib.h
+++ b/include/wimlib.h
@@ -2499,6 +2499,7 @@ enum wimlib_error_code {
 	WIMLIB_ERR_COMPACTION_NOT_POSSIBLE            = 85,
 	WIMLIB_ERR_IMAGE_HAS_MULTIPLE_REFERENCES      = 86,
 	WIMLIB_ERR_DUPLICATE_EXPORTED_IMAGE           = 87,
+	WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED   = 88,
 };
 
 
@@ -4474,12 +4475,13 @@ wimlib_update_image(WIMStruct *wim,
  *
  * @return 0 on success; a ::wimlib_error_code value on failure.
  *
+ * @retval ::WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED
+ * 	A file that had previously been scanned for inclusion in the WIM was
+ * 	concurrently modified.
  * @retval ::WIMLIB_ERR_INVALID_IMAGE
  * 	@p image did not exist in @p wim.
  * @retval ::WIMLIB_ERR_INVALID_RESOURCE_HASH
- * 	A file, stored in another WIM, which needed to be written was corrupt;
- * 	or a file that had previously been scanned for inclusion in the WIM was
- * 	concurrently modified.
+ * 	A file, stored in another WIM, which needed to be written was corrupt.
  * @retval ::WIMLIB_ERR_INVALID_PARAM
  * 	@p path was not a nonempty string, or invalid flags were passed.
  * @retval ::WIMLIB_ERR_OPEN
diff --git a/include/wimlib/blob_table.h b/include/wimlib/blob_table.h
index abc0c10f..09996600 100644
--- a/include/wimlib/blob_table.h
+++ b/include/wimlib/blob_table.h
@@ -387,6 +387,17 @@ blob_set_is_located_in_attached_buffer(struct blob_descriptor *blob,
 	blob->size = size;
 }
 
+static inline bool
+blob_is_in_file(const struct blob_descriptor *blob)
+{
+	return blob->blob_location == BLOB_IN_FILE_ON_DISK
+#ifdef __WIN32__
+	    || blob->blob_location == BLOB_IN_WINNT_FILE_ON_DISK
+	    || blob->blob_location == BLOB_WIN32_ENCRYPTED
+#endif
+	   ;
+}
+
 extern struct blob_descriptor *
 new_blob_from_data_buffer(const void *buffer, size_t size,
 			  struct blob_table *blob_table);
diff --git a/src/error.c b/src/error.c
index ea434912..5afaec1c 100644
--- a/src/error.c
+++ b/src/error.c
@@ -342,6 +342,8 @@ static const tchar * const error_strings[] = {
 		    "referenced from multiple places"),
 	[WIMLIB_ERR_DUPLICATE_EXPORTED_IMAGE]
 		= T("The destination WIM already contains one of the source images"),
+	[WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED]
+		= T("A file being added to a WIM image was concurrently modified"),
 };
 
 WIMLIBAPI const tchar *
diff --git a/src/resource.c b/src/resource.c
index 9afa25ff..72d05193 100644
--- a/src/resource.c
+++ b/src/resource.c
@@ -537,7 +537,8 @@ read_error:
  * data in nonempty chunks into the cbs->consume_chunk() function.  */
 static int
 read_raw_file_data(struct filedes *in_fd, u64 offset, u64 size,
-		   const struct read_blob_callbacks *cbs)
+		   const struct read_blob_callbacks *cbs,
+		   const tchar *filename)
 {
 	u8 buf[BUFFER_SIZE];
 	size_t bytes_to_read;
@@ -546,10 +547,8 @@ read_raw_file_data(struct filedes *in_fd, u64 offset, u64 size,
 	while (size) {
 		bytes_to_read = min(sizeof(buf), size);
 		ret = full_pread(in_fd, buf, bytes_to_read, offset);
-		if (unlikely(ret)) {
-			ERROR_WITH_ERRNO("Read error");
-			return ret;
-		}
+		if (unlikely(ret))
+			goto read_error;
 		ret = call_consume_chunk(buf, bytes_to_read, cbs);
 		if (unlikely(ret))
 			return ret;
@@ -557,6 +556,17 @@ read_raw_file_data(struct filedes *in_fd, u64 offset, u64 size,
 		offset += bytes_to_read;
 	}
 	return 0;
+
+read_error:
+	if (!filename) {
+		ERROR_WITH_ERRNO("Error reading data from WIM file");
+	} else if (ret == WIMLIB_ERR_UNEXPECTED_END_OF_FILE) {
+		ERROR("\"%"TS"\": File was concurrently truncated", filename);
+		ret = WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED;
+	} else {
+		ERROR_WITH_ERRNO("\"%"TS"\": Error reading data", filename);
+	}
+	return ret;
 }
 
 /* A consume_chunk() implementation that simply concatenates all chunks into an
@@ -601,7 +611,7 @@ read_partial_wim_resource(const struct wim_resource_descriptor *rdesc,
 	/* Uncompressed resource  */
 	return read_raw_file_data(&rdesc->wim->in_fd,
 				  rdesc->offset_in_wim + offset,
-				  size, cbs);
+				  size, cbs, NULL);
 }
 
 /* Read the specified range of uncompressed data from the specified blob, which
@@ -660,7 +670,7 @@ read_file_on_disk_prefix(const struct blob_descriptor *blob, u64 size,
 		return WIMLIB_ERR_OPEN;
 	}
 	filedes_init(&fd, raw_fd);
-	ret = read_raw_file_data(&fd, 0, size, cbs);
+	ret = read_raw_file_data(&fd, 0, size, cbs, blob->file_on_disk);
 	filedes_close(&fd);
 	return ret;
 }
@@ -682,7 +692,7 @@ read_staging_file_prefix(const struct blob_descriptor *blob, u64 size,
 		return WIMLIB_ERR_OPEN;
 	}
 	filedes_init(&fd, raw_fd);
-	ret = read_raw_file_data(&fd, 0, size, cbs);
+	ret = read_raw_file_data(&fd, 0, size, cbs, blob->staging_file_name);
 	filedes_close(&fd);
 	return ret;
 }
@@ -932,6 +942,62 @@ hasher_consume_chunk(const void *chunk, size_t size, void *_ctx)
 	return call_consume_chunk(chunk, size, &ctx->cbs);
 }
 
+static int
+report_sha1_mismatch_error(const struct blob_descriptor *blob,
+			   const u8 actual_hash[SHA1_HASH_SIZE])
+{
+	tchar expected_hashstr[SHA1_HASH_SIZE * 2 + 1];
+	tchar actual_hashstr[SHA1_HASH_SIZE * 2 + 1];
+
+	wimlib_assert(blob->blob_location != BLOB_NONEXISTENT);
+	wimlib_assert(blob->blob_location != BLOB_IN_ATTACHED_BUFFER);
+
+	sprint_hash(blob->hash, expected_hashstr);
+	sprint_hash(actual_hash, actual_hashstr);
+
+	if (blob_is_in_file(blob)) {
+		ERROR("A file was concurrently modified!\n"
+		      "        Path: \"%"TS"\"\n"
+		      "        Expected SHA-1: %"TS"\n"
+		      "        Actual SHA-1: %"TS"\n",
+		      blob->file_on_disk, expected_hashstr, actual_hashstr);
+		return WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED;
+	} else if (blob->blob_location == BLOB_IN_WIM) {
+		const struct wim_resource_descriptor *rdesc = blob->rdesc;
+		ERROR("A WIM resource is corrupted!\n"
+		      "        WIM file: \"%"TS"\"\n"
+		      "        Blob uncompressed size: %"PRIu64"\n"
+		      "        Resource offset in WIM: %"PRIu64"\n"
+		      "        Resource uncompressed size: %"PRIu64"\n"
+		      "        Resource size in WIM: %"PRIu64"\n"
+		      "        Resource flags: 0x%x%"TS"\n"
+		      "        Resource compression type: %"TS"\n"
+		      "        Resource compression chunk size: %"PRIu32"\n"
+		      "        Expected SHA-1: %"TS"\n"
+		      "        Actual SHA-1: %"TS"\n",
+		      rdesc->wim->filename,
+		      blob->size,
+		      rdesc->offset_in_wim,
+		      rdesc->uncompressed_size,
+		      rdesc->size_in_wim,
+		      (unsigned int)rdesc->flags,
+		      (rdesc->is_pipable ? T(", pipable") : T("")),
+		      wimlib_get_compression_type_string(
+						rdesc->compression_type),
+		      rdesc->chunk_size,
+		      expected_hashstr, actual_hashstr);
+		return WIMLIB_ERR_INVALID_RESOURCE_HASH;
+	} else {
+		ERROR("File data was concurrently modified!\n"
+		      "        Location ID: %d\n"
+		      "        Expected SHA-1: %"TS"\n"
+		      "        Actual SHA-1: %"TS"\n",
+		      (int)blob->blob_location,
+		      expected_hashstr, actual_hashstr);
+		return WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED;
+	}
+}
+
 /* Callback for finishing reading a blob while calculating its SHA-1 message
  * digest.  */
 static int
@@ -958,16 +1024,7 @@ hasher_end_blob(struct blob_descriptor *blob, int status, void *_ctx)
 	} else if ((ctx->flags & VERIFY_BLOB_HASHES) &&
 		   unlikely(!hashes_equal(hash, blob->hash)))
 	{
-		if (wimlib_print_errors) {
-			tchar expected_hashstr[SHA1_HASH_SIZE * 2 + 1];
-			tchar actual_hashstr[SHA1_HASH_SIZE * 2 + 1];
-			sprint_hash(blob->hash, expected_hashstr);
-			sprint_hash(hash, actual_hashstr);
-			ERROR("The data is corrupted!\n"
-			      "        (Expected SHA-1=%"TS", got SHA-1=%"TS")",
-			      expected_hashstr, actual_hashstr);
-		}
-		ret = WIMLIB_ERR_INVALID_RESOURCE_HASH;
+		ret = report_sha1_mismatch_error(blob, hash);
 		goto out_next_cb;
 	}
 	ret = 0;
diff --git a/src/win32_capture.c b/src/win32_capture.c
index 88e31248..0f608a8f 100644
--- a/src/win32_capture.c
+++ b/src/win32_capture.c
@@ -137,10 +137,16 @@ read_winnt_stream_prefix(const struct blob_descriptor *blob, u64 size,
 
 		status = (*func_NtReadFile)(h, NULL, NULL, NULL,
 					    &iosb, buf, count, NULL, NULL);
-		if (!NT_SUCCESS(status)) {
-			winnt_error(status, L"\"%ls\": Error reading data",
-				    printable_path(path));
-			ret = WIMLIB_ERR_READ;
+		if (unlikely(!NT_SUCCESS(status))) {
+			if (status == STATUS_END_OF_FILE) {
+				ERROR("\"%ls\": File was concurrently truncated",
+				      printable_path(path));
+				ret = WIMLIB_ERR_CONCURRENT_MODIFICATION_DETECTED;
+			} else {
+				winnt_error(status, L"\"%ls\": Error reading data",
+					    printable_path(path));
+				ret = WIMLIB_ERR_READ;
+			}
 			break;
 		}
 
diff --git a/src/write.c b/src/write.c
index 23dda573..e118ac44 100644
--- a/src/write.c
+++ b/src/write.c
@@ -1340,17 +1340,6 @@ validate_blob_list(struct list_head *blob_list)
 	}
 }
 
-static inline bool
-blob_is_in_file(const struct blob_descriptor *blob)
-{
-	return blob->blob_location == BLOB_IN_FILE_ON_DISK
-#ifdef __WIN32__
-	    || blob->blob_location == BLOB_IN_WINNT_FILE_ON_DISK
-	    || blob->blob_location == BLOB_WIN32_ENCRYPTED
-#endif
-	   ;
-}
-
 static void
 init_done_with_file_info(struct list_head *blob_list)
 {