From bc1a4ee900196f4934a79b434aa3c3ca24e65a23 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 16 Dec 2016 19:47:44 -0800 Subject: [PATCH] join.c: clean up verify_swm_set() UBSAN complained when the parts_to_swms array had 0 length. Clean this up by sorting the parts first, making the verification simpler. Also don't bother checking compression_type and chunk_size anymore; checking guid should be sufficient, and it doesn't really matter if the compression formats are different since now everything will be written out correctly anyway. --- src/join.c | 191 ++++++++++++++--------------------------------------- 1 file changed, 50 insertions(+), 141 deletions(-) diff --git a/src/join.c b/src/join.c index cca44214..27e7a02c 100644 --- a/src/join.c +++ b/src/join.c @@ -5,7 +5,7 @@ */ /* - * Copyright (C) 2012, 2013 Eric Biggers + * Copyright (C) 2012-2016 Eric Biggers * * This file is free software; you can redistribute it and/or modify it under * the terms of the GNU Lesser General Public License as published by the Free @@ -25,6 +25,8 @@ # include "config.h" #endif +#include + #include "wimlib.h" #include "wimlib/error.h" #include "wimlib/types.h" @@ -32,121 +34,46 @@ #include "wimlib/wim.h" /* - * verify_swm_set: - Sanity checks to make sure a set of WIMs correctly - * correspond to a spanned set. - * - * @wim: - * Part 1 of the set. - * - * @additional_swms: - * All parts of the set other than part 1. + * Verify that a list of WIM files sorted by part number is a spanned set. * - * @num_additional_swms: - * Number of WIMStructs in @additional_swms. Or, the total number of parts - * in the set minus 1. - * - * @return: - * 0 on success; WIMLIB_ERR_SPLIT_INVALID if the set is not valid. + * Return: 0 on success; WIMLIB_ERR_SPLIT_INVALID if the set is not valid. */ static int -verify_swm_set(WIMStruct *wim, WIMStruct **additional_swms, - unsigned num_additional_swms) +verify_swm_set(WIMStruct * const *swms, unsigned num_swms) { - unsigned total_parts = wim->hdr.total_parts; - int ctype; - u32 chunk_size; - const u8 *guid; - - if (total_parts != num_additional_swms + 1) { - ERROR("`%"TS"' says there are %u parts in the spanned set, " - "but %"TS"%u part%"TS" provided", - wim->filename, total_parts, - (num_additional_swms + 1 < total_parts) ? T("only ") : T(""), - num_additional_swms + 1, - (num_additional_swms) ? T("s were") : T(" was")); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (wim->hdr.part_number != 1) { - ERROR("WIM `%"TS"' is not the first part of the split WIM.", - wim->filename); - return WIMLIB_ERR_SPLIT_INVALID; - } - for (unsigned i = 0; i < num_additional_swms; i++) { - if (additional_swms[i]->hdr.total_parts != total_parts) { - ERROR("WIM `%"TS"' says there are %u parts in the " - "spanned set, but %u parts were provided", - additional_swms[i]->filename, - additional_swms[i]->hdr.total_parts, - total_parts); + for (unsigned i = 0; i < num_swms; i++) { + if (!guids_equal(swms[i]->hdr.guid, swms[0]->hdr.guid)) { + ERROR("The split WIM parts specified belong to " + "different split WIMs!"); return WIMLIB_ERR_SPLIT_INVALID; } - } - - /* Keep track of the compression type, chunk size, and GUID to make sure - * they are the same for all the WIMs. */ - ctype = wim->compression_type; - chunk_size = wim->chunk_size; - guid = wim->hdr.guid; - - { - /* parts_to_swms is not allocated at function scope because it - * should only be allocated after num_additional_swms was - * checked to be the same as wim->hdr.total_parts. Otherwise, it - * could be unexpectedly high and cause a stack overflow. */ - WIMStruct *parts_to_swms[num_additional_swms]; - memset(parts_to_swms, 0, sizeof(parts_to_swms)); - for (unsigned i = 0; i < num_additional_swms; i++) { - - WIMStruct *swm = additional_swms[i]; - - if (swm->compression_type != ctype) { - ERROR("The split WIMs do not all have the same " - "compression type"); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (swm->chunk_size != chunk_size && - ctype != WIMLIB_COMPRESSION_TYPE_NONE) { - ERROR("The split WIMs do not all have the same " - "chunk size"); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (!guids_equal(guid, swm->hdr.guid)) { - ERROR("The split WIMs do not all have the same " - "GUID"); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (swm->hdr.part_number == 1) { - ERROR("WIMs `%"TS"' and `%"TS"' both are marked " - "as the first WIM in the spanned set", - wim->filename, swm->filename); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (swm->hdr.part_number == 0 || - swm->hdr.part_number > total_parts) - { - ERROR("WIM `%"TS"' says it is part %u in the " - "spanned set, but the part number must " - "be in the range [1, %u]", - swm->filename, swm->hdr.part_number, total_parts); - return WIMLIB_ERR_SPLIT_INVALID; - } - if (parts_to_swms[swm->hdr.part_number - 2]) - { - ERROR("`%"TS"' and `%"TS"' are both marked as " - "part %u of %u in the spanned set", - parts_to_swms[swm->hdr.part_number - 2]->filename, - swm->filename, - swm->hdr.part_number, - total_parts); - return WIMLIB_ERR_SPLIT_INVALID; - } else { - parts_to_swms[swm->hdr.part_number - 2] = swm; - } + if (swms[i]->hdr.total_parts != num_swms) { + ERROR("\"%"TS"\" says there are %u parts in the split " + "WIM, but %s%u part%s provided", + swms[i]->filename, swms[i]->hdr.total_parts, + num_swms < swms[i]->hdr.total_parts ? "only ":"", + num_swms, num_swms > 1 ? "s were" : " was"); + return WIMLIB_ERR_SPLIT_INVALID; + } + if (swms[i]->hdr.part_number != i + 1) { + ERROR("The parts of the split WIM are not numbered " + "1..%u as expected. Did you specify duplicate " + "parts?", num_swms); + return WIMLIB_ERR_SPLIT_INVALID; } } return 0; } +static int +cmp_swms_by_part_number(const void *p1, const void *p2) +{ + WIMStruct *swm1 = *(WIMStruct **)p1; + WIMStruct *swm2 = *(WIMStruct **)p2; + + return (int)swm1->hdr.part_number - (int)swm2->hdr.part_number; +} + WIMLIBAPI int wimlib_join_with_progress(const tchar * const *swm_names, unsigned num_swms, @@ -156,67 +83,49 @@ wimlib_join_with_progress(const tchar * const *swm_names, wimlib_progress_func_t progfunc, void *progctx) { - int ret; + WIMStruct **swms; unsigned i; - unsigned j; - WIMStruct *swm0; - WIMStruct **additional_swms; - unsigned num_additional_swms; + int ret; if (num_swms < 1 || num_swms > 0xffff) return WIMLIB_ERR_INVALID_PARAM; - num_additional_swms = num_swms - 1; - additional_swms = CALLOC((num_additional_swms + 1), - sizeof(additional_swms[0])); - if (!additional_swms) + swms = CALLOC(num_swms, sizeof(swms[0])); + if (!swms) return WIMLIB_ERR_NOMEM; - swm0 = NULL; - for (i = 0, j = 0; i < num_swms; i++) { - WIMStruct *swm; - + for (i = 0; i < num_swms; i++) { ret = wimlib_open_wim_with_progress(swm_names[i], swm_open_flags, - &swm, + &swms[i], progfunc, progctx); if (ret) - goto out_free_swms; - if (swm->hdr.part_number == 1 && swm0 == NULL) - swm0 = swm; - else - additional_swms[j++] = swm; + goto out; } - if (!swm0) { - ERROR("Part 1 of the split WIM was not specified!"); - ret = WIMLIB_ERR_SPLIT_INVALID; - goto out_free_swms; - } + qsort(swms, num_swms, sizeof(swms[0]), cmp_swms_by_part_number); - ret = verify_swm_set(swm0, additional_swms, num_additional_swms); + ret = verify_swm_set(swms, num_swms); if (ret) - goto out_free_swms; + goto out; - ret = wimlib_reference_resources(swm0, additional_swms, - num_additional_swms, 0); + ret = wimlib_reference_resources(swms[0], swms + 1, num_swms - 1, 0); if (ret) - goto out_free_swms; + goto out; - /* It is reasonably safe to provide, WIMLIB_WRITE_FLAG_STREAMS_OK, as we + /* It is reasonably safe to provide WIMLIB_WRITE_FLAG_STREAMS_OK, as we * have verified that the specified split WIM parts form a spanned set. */ - ret = wimlib_write(swm0, output_path, WIMLIB_ALL_IMAGES, + ret = wimlib_write(swms[0], output_path, WIMLIB_ALL_IMAGES, wim_write_flags | WIMLIB_WRITE_FLAG_STREAMS_OK | WIMLIB_WRITE_FLAG_RETAIN_GUID, 1); -out_free_swms: - for (i = 0; i < num_additional_swms + 1; i++) - wimlib_free(additional_swms[i]); - FREE(additional_swms); - wimlib_free(swm0); +out: + for (i = 0; i < num_swms; i++) + wimlib_free(swms[i]); + FREE(swms); return ret; } -- 2.43.0