From 039ed8e4985f5a1414bb8d2dccd1c378d83e60f2 Mon Sep 17 00:00:00 2001 From: Ilan Truanovsky Date: Mon, 16 Jan 2023 11:08:02 -0800 Subject: [PATCH] Prevent zlib from being dynamically loaded A pointer containing the dynamically loaded zlib library was not being properly cleaned up. This is picked up by our Coverity scans. One solution to this would be to store the handle to the library alongside the function pointers to function within that library. Then, we can free the library whenever we call `inflateEnd` or `deflateEnd`. However, this solution ended up not being necessary. Another solution (the one this commit implements) removes zlib from being dynamically loaded in the first place. This is possible since we already link zlib with the runtime during compilation, and so loading zlib dynamically is unnecessary. This fixes the following Coverity issue: ``` lib/pkg_editor/src/zlib.c:119:1: Type: Resource leak (RESOURCE_LEAK) lib/pkg_editor/src/zlib.c:98:3: 1. path: Condition "zlib_interface.deflateInit_", taking false branch. lib/pkg_editor/src/zlib.c:101:3: 2. alloc_fn: Storage is returned from allocation function "zlib_load_library". lib/pkg_editor/src/zlib.c:90:3: 2.1. path: Condition "dlopen_handle", taking false branch. lib/pkg_editor/src/zlib.c:93:3: 2.2. alloc_fn: Storage is returned from allocation function "dlopen". lib/pkg_editor/src/zlib.c:93:3: 2.3. return_alloc_fn: Directly returning storage allocated by "dlopen". lib/pkg_editor/src/zlib.c:101:3: 3. var_assign: Assigning: "library" = storage returned from "zlib_load_library()". lib/pkg_editor/src/zlib.c:102:3: 4. path: Condition "library == NULL", taking false branch. lib/pkg_editor/src/zlib.c:110:3: 5. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 5.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:111:3: 6. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 6.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:112:3: 7. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 7.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:113:3: 8. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 8.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:114:3: 9. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 9.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:115:3: 10. noescape: Resource "library" is not freed or pointed-to in "load_one_symbol". lib/pkg_editor/src/zlib.c:39:36: 10.1. noescape: "load_one_symbol(void *, char const *)" does not free or save its parameter "library". lib/pkg_editor/src/zlib.c:119:1: 11. leaked_storage: Variable "library" going out of scope leaks the storage it points to. ``` --- lib/pkg_editor/CMakeLists.txt | 1 - lib/pkg_editor/src/pkg_editor.c | 59 ++++------- lib/pkg_editor/src/zlib.c | 150 ---------------------------- lib/pkg_editor/src/zlib_interface.h | 30 ------ 4 files changed, 20 insertions(+), 220 deletions(-) delete mode 100644 lib/pkg_editor/src/zlib.c delete mode 100644 lib/pkg_editor/src/zlib_interface.h diff --git a/lib/pkg_editor/CMakeLists.txt b/lib/pkg_editor/CMakeLists.txt index b26e49c6..e87fdcc2 100644 --- a/lib/pkg_editor/CMakeLists.txt +++ b/lib/pkg_editor/CMakeLists.txt @@ -20,7 +20,6 @@ if(ZLIB_FOUND) target_compile_definitions(pkg_editor PRIVATE WINDOWS_ZLIB_PATH="${WINDOWS_ZLIB_DLL_PATH}") endif() target_link_libraries(pkg_editor PRIVATE ZLIB::ZLIB) - target_sources(pkg_editor PRIVATE src/zlib.c) endif() install(TARGETS pkg_editor diff --git a/lib/pkg_editor/src/pkg_editor.c b/lib/pkg_editor/src/pkg_editor.c index 3bfa921c..b7d47598 100644 --- a/lib/pkg_editor/src/pkg_editor.c +++ b/lib/pkg_editor/src/pkg_editor.c @@ -39,9 +39,7 @@ #endif #include "pkg_editor/pkg_editor.h" -#if USE_ZLIB -#include "zlib_interface.h" -#endif +#include "zlib.h" typedef struct acl_pkg_file { const char *fname; @@ -1194,7 +1192,7 @@ static int append_data(const void *data, size_t size, ZInfo *z_info, FILE *of, int ret; z_info->strm.avail_out = sizeof(z_info->buffer); z_info->strm.next_out = z_info->buffer; - ret = zlib_deflate(&z_info->strm, Z_FINISH); + ret = deflate(&z_info->strm, Z_FINISH); assert(ret != Z_STREAM_ERROR); output_size = sizeof(z_info->buffer) - z_info->strm.avail_out; if (output_size > 0) { @@ -1206,7 +1204,7 @@ static int append_data(const void *data, size_t size, ZInfo *z_info, FILE *of, } else { // Only dump the output buffer when it is full. do { - int ret = zlib_deflate(&z_info->strm, Z_NO_FLUSH); + int ret = deflate(&z_info->strm, Z_NO_FLUSH); assert(ret != Z_STREAM_ERROR); if (z_info->strm.avail_out == 0) { if (fwrite(z_info->buffer, sizeof(z_info->buffer), 1, of) != 1) { @@ -1476,7 +1474,8 @@ int acl_pkg_pack(const char *out_file, const char **input_files_dirs) { z_info.strm.opaque = Z_NULL; z_info.strm.avail_out = sizeof(z_info.buffer); z_info.strm.next_out = z_info.buffer; - ret = zlib_deflateInit(&z_info.strm, Z_BEST_COMPRESSION); + ret = deflateInit_(&z_info.strm, Z_BEST_COMPRESSION, ZLIB_VERSION, + (int)sizeof(z_stream)); if (ret != Z_OK) { fprintf(stderr, "acl_pkg_pack: Unable to initialize zlib for writing %s\n", out_file); @@ -1491,7 +1490,7 @@ int acl_pkg_pack(const char *out_file, const char **input_files_dirs) { if (result == PACK_END) { // We had a failure; stop here. fclose(of); - zlib_deflateEnd(&z_info.strm); + deflateEnd(&z_info.strm); return 0; } input_files_dirs++; @@ -1505,10 +1504,10 @@ int acl_pkg_pack(const char *out_file, const char **input_files_dirs) { if (fclose(of) != 0) { fprintf(stderr, "acl_pkg_pack: Write of %s failed: %s\n", out_file, strerror(errno)); - zlib_deflateEnd(&z_info.strm); + deflateEnd(&z_info.strm); return 0; } - zlib_deflateEnd(&z_info.strm); + deflateEnd(&z_info.strm); return 1 /* success */; } @@ -1534,7 +1533,7 @@ static int read_data(void *data, size_t size, ZInfo *z_info, FILE *in_fd) { z_info->strm.next_in = z_info->buffer; } // Grab the next chunk of data from the input buffer. - ret = zlib_inflate(&z_info->strm, Z_NO_FLUSH); + ret = inflate(&z_info->strm, Z_NO_FLUSH); assert(ret != Z_STREAM_ERROR); if (ret == Z_STREAM_END) { // Last bit of data. @@ -1590,7 +1589,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, z_info.strm.avail_in = 0; z_info.strm.next_in = NULL; } - ret = zlib_inflateInit(&z_info.strm); + ret = inflateInit_(&z_info.strm, ZLIB_VERSION, (int)sizeof(z_stream)); if (ret != Z_OK) { fprintf(stderr, "%s: Unable to initialize zlib for reading from buffer\n", routine_name); @@ -1611,13 +1610,13 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, acl_pkg_pack_info info; if (!read_data(&info, sizeof(info), &z_info, input)) { fprintf(stderr, "%s: Error reading from buffer\n", routine_name); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } if (info.magic != PACK_MAGIC) { fprintf(stderr, "%s: Incorrect magic number read from buffer\n", routine_name); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } @@ -1630,7 +1629,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, if (!read_data(name, info.name_length, &z_info, input)) { fprintf(stderr, "%s: Error reading file name from buffer\n", routine_name); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } @@ -1656,7 +1655,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, if (out_file == NULL) { fprintf(stderr, "%s: Unable to open %s for writing: %s\n", routine_name, full_name, strerror(errno)); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } if (info.file_length > 0) { @@ -1666,14 +1665,14 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, fprintf(stderr, "%s: Error reading file data for %s from buffer\n", routine_name, full_name); fclose(out_file); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } if (fwrite(buf, info.file_length, 1, out_file) != 1) { fprintf(stderr, "%s: Failed to write to %s: %s\n", routine_name, full_name, strerror(errno)); fclose(out_file); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } } else { @@ -1683,7 +1682,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, routine_name, full_name, strerror(errno)); fclose(out_file); free(buf2); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return PACK_END; } if (!read_data(buf2, info.file_length, &z_info, input)) { @@ -1691,7 +1690,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, routine_name, full_name); fclose(out_file); free(buf2); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } if (fwrite(buf2, info.file_length, 1, out_file) != 1) { @@ -1699,7 +1698,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, full_name, strerror(errno)); fclose(out_file); free(buf2); - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 0; } free(buf2); @@ -1709,7 +1708,7 @@ static int acl_pkg_unpack_buffer_or_file(const char *buffer, size_t buffer_size, } } - zlib_inflateEnd(&z_info.strm); + inflateEnd(&z_info.strm); return 1; } @@ -1744,22 +1743,4 @@ int acl_pkg_unpack(const char *in_file, const char *out_dir) { return ret; } -#else // USE_ZLIB - -int acl_pkg_pack(const char *out_file, const char **input_files_dirs) { - // Not implemented if no ZLIB - return 0; -} - -int acl_pkg_unpack(const char *in_file, const char *out_dir) { - // Not implemented if no ZLIB - return 0; -} - -int acl_pkg_unpack_buffer(const char *buffer, size_t buffer_size, - const char *out_dir) { - // Not implemented if no ZLIB - return 0; -} - #endif // USE_ZLIB diff --git a/lib/pkg_editor/src/zlib.c b/lib/pkg_editor/src/zlib.c deleted file mode 100644 index c5f23d84..00000000 --- a/lib/pkg_editor/src/zlib.c +++ /dev/null @@ -1,150 +0,0 @@ -// Copyright (C) 2017-2021 Intel Corporation -// SPDX-License-Identifier: BSD-3-Clause - -/* Interface to Zlib DLL - */ - -#include -#include -#include -#include -#include -#include -#ifdef _WIN32 -#include -#else -#include -#include -#include -#endif - -#include "zlib_interface.h" -#ifdef __cplusplus -extern "C" { -#endif - -typedef int (*zlib_stream)(z_streamp); -typedef int (*zlib_stream_int)(z_streamp, int); -typedef struct ZlibInterface { - int (*deflateInit_)(z_streamp, int, const char *version, int stream_size); - zlib_stream_int deflate; - zlib_stream deflateEnd; - int (*inflateInit_)(z_streamp, const char *version, int stream_size); - zlib_stream_int inflate; - zlib_stream inflateEnd; -} ZlibInterface; - -static ZlibInterface zlib_interface; - -static void *load_one_symbol(void *library, const char *function_name) { - void *symbol; -#ifdef _MSC_VER -#pragma warning(push) -#pragma warning(disable : 4152) - symbol = GetProcAddress((HMODULE)library, function_name); -#pragma warning(pop) -#else - symbol = dlsym(library, function_name); - if (symbol == NULL) { - fprintf(stderr, "Unable to find dynamic symbol %s: %s\n", function_name, - dlerror()); - exit(1); - } -#endif - return symbol; -} - -#ifdef _WIN32 -// Retrieve zlib path from INTELFPGAOCLSDKROOT -// The caller must free the returned buffer after use! -static char *zlib_get_path_fpga_root(const char *acl_root_dir) { - const char *const zlib_rel_path = "\\windows64\\bin\\zlib1.dll"; - char *const zlib_path = - malloc(strlen(acl_root_dir) + strlen(zlib_rel_path) + 1); - if (!zlib_path) { - fprintf(stderr, "Failed to allocate memory for the zlib path!\n"); - exit(1); - } - strcpy(zlib_path, acl_root_dir); - strcat(zlib_path, zlib_rel_path); - return zlib_path; -} -#endif - -// Dynamically loads zlib library and returns handle on success, or `NULL` on -// failure. -static void *zlib_load_library() { -#ifdef _WIN32 - const char *const acl_root_dir = getenv("INTELFPGAOCLSDKROOT"); - if (acl_root_dir) { - char *const zlib_path = zlib_get_path_fpga_root(acl_root_dir); - void *const library = (void *)LoadLibraryA(zlib_path); - free(zlib_path); - return library; - } - // Standalone runtime, INTELFPGAOCLSDKROOT is not set. - void *const library = (void *)LoadLibraryA(WINDOWS_ZLIB_PATH); - return library; -#else - void *const dlopen_handle = dlopen("libz.so.1", RTLD_NOW); - if (dlopen_handle) { - return dlopen_handle; - } - return dlopen("libz.so", RTLD_NOW); -#endif -} - -static void zlib_load() { - if (zlib_interface.deflateInit_) { - return; // library is already loaded - } - void *const library = zlib_load_library(); - if (library == NULL) { - fprintf(stderr, "Unable to open zlib library!\n"); - exit(1); - } -#ifdef _MSC_VER -#pragma warning(push) -#pragma warning(disable : 4152) -#endif - zlib_interface.deflateInit_ = load_one_symbol(library, "deflateInit_"); - zlib_interface.deflate = load_one_symbol(library, "deflate"); - zlib_interface.deflateEnd = load_one_symbol(library, "deflateEnd"); - zlib_interface.inflateInit_ = load_one_symbol(library, "inflateInit_"); - zlib_interface.inflate = load_one_symbol(library, "inflate"); - zlib_interface.inflateEnd = load_one_symbol(library, "inflateEnd"); -#ifdef _MSC_VER -#pragma warning(pop) -#endif -} - -int zlib_deflateInit(z_streamp strm, int level) { - zlib_load(); - return (*zlib_interface.deflateInit_)(strm, level, ZLIB_VERSION, - (int)sizeof(z_stream)); -} - -int zlib_deflate(z_streamp strm, int flush) { - return (*zlib_interface.deflate)(strm, flush); -} - -int zlib_deflateEnd(z_streamp strm) { - return (*zlib_interface.deflateEnd)(strm); -} - -int zlib_inflateInit(z_streamp strm) { - zlib_load(); - return (*zlib_interface.inflateInit_)(strm, ZLIB_VERSION, - (int)sizeof(z_stream)); -} - -int zlib_inflate(z_streamp strm, int flush) { - return (*zlib_interface.inflate)(strm, flush); -} -int zlib_inflateEnd(z_streamp strm) { - return (*zlib_interface.inflateEnd)(strm); -} - -#ifdef __cplusplus -} -#endif diff --git a/lib/pkg_editor/src/zlib_interface.h b/lib/pkg_editor/src/zlib_interface.h deleted file mode 100644 index b30daec2..00000000 --- a/lib/pkg_editor/src/zlib_interface.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (C) 2017-2021 Intel Corporation -// SPDX-License-Identifier: BSD-3-Clause - -/* Interface to zlib for Package editor - * - * This dynamically loads zlib, avoiding build dependencies. - */ - -#ifndef ZLIB_INTERFACE_H -#define ZLIB_INTERFACE_H - -#include "zlib.h" - -#ifdef __cplusplus -extern "C" { -#endif - -// Interface to zlib routines, handling DLL loading first. -int zlib_deflateInit(z_streamp, int); -int zlib_deflate(z_streamp, int); -int zlib_deflateEnd(z_streamp); -int zlib_inflateInit(z_streamp); -int zlib_inflate(z_streamp, int); -int zlib_inflateEnd(z_streamp); - -#ifdef __cplusplus -} -#endif - -#endif /* ZLIB_INTERFACE_H */