From 48d7c4e0f3626a744bfa4434b3fd663d71c5824a Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Fri, 16 Oct 2020 15:07:34 +1100 Subject: [PATCH] util/disk_cache: do crc32 check on compressed data for ZSTD This will be faster and avoids checking for errors with the compression implementation which we shouldn't need to do. Instead we trust the compression library does the correct thing and simply error check the data loaded from disk. Reviewed-by: Pierre-Eric Pelloux-Prayer Part-of: --- src/util/disk_cache.c | 9 +------- src/util/disk_cache_os.c | 55 +++++++++++++++++++++++++++++++++++++++--------- src/util/disk_cache_os.h | 6 ------ 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c index 402991a..bf77fe0 100644 --- a/src/util/disk_cache.c +++ b/src/util/disk_cache.c @@ -322,14 +322,7 @@ cache_put(void *job, int thread_index) i++; } - /* Create CRC of the data. We will read this when restoring the cache and - * use it to check for corruption. - */ - struct cache_entry_file_data cf_data; - cf_data.crc32 = util_hash_crc32(dc_job->data, dc_job->size); - cf_data.uncompressed_size = dc_job->size; - - disk_cache_write_item_to_disk(dc_job, &cf_data, filename); + disk_cache_write_item_to_disk(dc_job, filename); done: free(filename); diff --git a/src/util/disk_cache_os.c b/src/util/disk_cache_os.c index c4ee186..52de69e 100644 --- a/src/util/disk_cache_os.c +++ b/src/util/disk_cache_os.c @@ -39,6 +39,13 @@ #include "zstd.h" #endif +#include "util/crc32.h" + +struct cache_entry_file_data { + uint32_t crc32; + uint32_t uncompressed_size; +}; + /* 3 is the recomended level, with 22 as the absolute maximum */ #define ZSTD_COMPRESSION_LEVEL 3 @@ -71,7 +78,22 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest) free(out); return 0; } - ssize_t written = write_all(dest, out, ret); + + /* Create CRC of the compressed data. We will read this when restoring the + * cache and use it to check for corruption. + */ + struct cache_entry_file_data cf_data; + cf_data.crc32 = util_hash_crc32(out, ret); + cf_data.uncompressed_size = in_data_size; + + size_t cf_data_size = sizeof(cf_data); + ssize_t written = write_all(dest, &cf_data, cf_data_size); + if (written == -1) { + free(out); + return 0; + } + + written = write_all(dest, out, ret); if (written == -1) { free(out); return 0; @@ -79,6 +101,19 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest) free(out); return ret; #else + /* Create CRC of the uncompressed data. We will read this when restoring + * the cache and use it to check for corruption. + */ + struct cache_entry_file_data cf_data; + cf_data.crc32 = util_hash_crc32(in_data, in_data_size); + cf_data.uncompressed_size = in_data_size; + + size_t cf_data_size = sizeof(cf_data); + ssize_t written = write_all(dest, &cf_data, cf_data_size); + if (written == -1) { + return 0; + } + unsigned char *out; /* allocate deflate state */ @@ -119,7 +154,7 @@ deflate_and_write_to_disk(const void *in_data, size_t in_data_size, int dest) size_t have = BUFSIZE - strm.avail_out; compressed_size += have; - ssize_t written = write_all(dest, out, have); + written = write_all(dest, out, have); if (written == -1) { (void)deflateEnd(&strm); free(out); @@ -595,16 +630,24 @@ disk_cache_load_item(struct disk_cache *cache, char *filename, size_t *size) if (ret == -1) goto fail; +#ifdef HAVE_ZSTD + /* Check the data for corruption */ + if (cf_data.crc32 != util_hash_crc32(data, cache_data_size)) + goto fail; +#endif + /* Uncompress the cache data */ uncompressed_data = malloc(cf_data.uncompressed_size); if (!inflate_cache_data(data, cache_data_size, uncompressed_data, cf_data.uncompressed_size)) goto fail; +#ifndef HAVE_ZSTD /* Check the data for corruption */ if (cf_data.crc32 != util_hash_crc32(uncompressed_data, cf_data.uncompressed_size)) goto fail; +#endif free(data); free(filename); @@ -654,7 +697,6 @@ disk_cache_get_cache_filename(struct disk_cache *cache, const cache_key key) void disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job, - struct cache_entry_file_data *cf_data, char *filename) { int fd = -1, fd_final = -1; @@ -756,13 +798,6 @@ disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job, } } - size_t cf_data_size = sizeof(*cf_data); - ret = write_all(fd, cf_data, cf_data_size); - if (ret == -1) { - unlink(filename_tmp); - goto done; - } - /* Now, finally, write out the contents to the temporary file, then * rename them atomically to the destination filename, and also * perform an atomic increment of the total cache size. diff --git a/src/util/disk_cache_os.h b/src/util/disk_cache_os.h index 490e807..7ac6a51 100644 --- a/src/util/disk_cache_os.h +++ b/src/util/disk_cache_os.h @@ -89,11 +89,6 @@ struct disk_cache_put_job { struct cache_item_metadata cache_item_metadata; }; -struct cache_entry_file_data { - uint32_t crc32; - uint32_t uncompressed_size; -}; - char * disk_cache_generate_cache_dir(void *mem_ctx); @@ -111,7 +106,6 @@ disk_cache_get_cache_filename(struct disk_cache *cache, const cache_key key); void disk_cache_write_item_to_disk(struct disk_cache_put_job *dc_job, - struct cache_entry_file_data *cf_data, char *filename); bool -- 2.7.4