From b053a2c1e4e50df76aeb1a3b832225f32d4266cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 19 Jul 2024 09:09:58 +0200 Subject: [PATCH 1/4] Add a cmake option to build with sanitizers Disabled by default, only for debugging. When using it it may be required to manually preload: `LD_PRELOAD=/usr/lib64/libasan.so.8` for the client process. This is because ASan runtime requires to be first in initial library list. --- CMakeLists.txt | 7 +++++++ librepo.spec | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 166ac03f..68ce9537 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,7 @@ OPTION (ENABLE_PYTHON "Build Python bindings" ON) OPTION (USE_GPGME "Use GpgMe (instead of rpm library) for OpenPGP key support" ON) OPTION (USE_RUN_GNUPG_USER_SOCKET "Create a directory for gpg-agent socket in /run/gnugp/user (instead of /run/user)" OFF) OPTION (ENABLE_SELINUX "Restore SELinux labels on GnuPG directories" ON) +OPTION (WITH_SANITIZERS "Build with address, leak and undefined sanitizers (DEBUG ONLY)" OFF) INCLUDE (${CMAKE_SOURCE_DIR}/VERSION.cmake) SET (VERSION "${LIBREPO_MAJOR}.${LIBREPO_MINOR}.${LIBREPO_PATCH}") @@ -95,6 +96,12 @@ IF (NOT CURL_FOUND) MESSAGE(FATAL_ERROR "No CURL library installed") ENDIF (NOT CURL_FOUND) +IF (WITH_SANITIZERS) + MESSAGE(WARNING "Building with sanitizers enabled!") + ADD_COMPILE_OPTIONS(-fsanitize=address -fsanitize=undefined -fsanitize=leak) + LINK_LIBRARIES(asan ubsan) +ENDIF() + # Generate header files with the configured ABI (e.g. with/without zchunk). configure_file("librepo/downloadtarget.h.in" "librepo/downloadtarget.h" @ONLY) diff --git a/librepo.spec b/librepo.spec index 4608dea9..071d2d42 100644 --- a/librepo.spec +++ b/librepo.spec @@ -19,6 +19,8 @@ # Needs to match how gnupg2 is compiled %bcond_with run_gnupg_user_socket +%bcond_with sanitizers + %if %{with use_gpgme} && %{with use_selinux} %global need_selinux 1 %else @@ -59,6 +61,12 @@ BuildRequires: pkgconfig(zck) >= 0.9.11 %endif Requires: libcurl%{?_isa} >= %{libcurl_version} +%if %{with sanitizers} +BuildRequires: libasan +BuildRequires: liblsan +BuildRequires: libubsan +%endif + %description A library providing C and Python (libcURL like) API to downloading repository metadata. @@ -97,6 +105,7 @@ Python 3 bindings for the librepo library. -DWITH_ZCHUNK=%{?with_zchunk:ON}%{!?with_zchunk:OFF} \ -DUSE_GPGME=%{?with_use_gpgme:ON}%{!?with_use_gpgme:OFF} \ -DUSE_RUN_GNUPG_USER_SOCKET=%{?with_run_gnupg_user_socket:ON}%{!?with_run_gnupg_user_socket:OFF} \ + -DWITH_SANITIZERS=%{?with_sanitizers:ON}%{!?with_sanitizers:OFF} \ %if %{need_selinux} -DENABLE_SELINUX=ON %else From 2fd52f236723f110036d13703f5477d0060392b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 19 Jul 2024 09:58:05 +0200 Subject: [PATCH 2/4] Free leaking `zck_src` if `zck_init_read` fails --- librepo/downloader.c | 1 + 1 file changed, 1 insertion(+) diff --git a/librepo/downloader.c b/librepo/downloader.c index 40dbeb26..1d696539 100644 --- a/librepo/downloader.c +++ b/librepo/downloader.c @@ -1228,6 +1228,7 @@ find_local_zck_chunks(LrTarget *target, GError **err) zckCtx *zck_src = zck_create(); if(!zck_init_read(zck_src, chk_fd)) { + zck_free(&zck_src); close(chk_fd); continue; } From 1e15c186ba482326f7a9fb9e0972bfa683f9dcbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 19 Jul 2024 11:10:20 +0200 Subject: [PATCH 3/4] Free `zckCtx` from `zckDL` before overwriting it Since the `zckCtx` from `zckDL` is referenced only in `zckDL` and there is only a single `zckDL` per `LrDownloadTarget` we have to free previous `zckCtx` before overwriting it. Otherwise it leaks. --- librepo/downloader.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/librepo/downloader.c b/librepo/downloader.c index 1d696539..7208a407 100644 --- a/librepo/downloader.c +++ b/librepo/downloader.c @@ -1139,6 +1139,8 @@ prep_zck_header(LrTarget *target, GError **err) zck = lr_zck_init_read(target->target, target->target->path, fd, &tmp_err); if(zck) { + zckCtx *old_zck = zck_dl_get_zck(target->target->zck_dl); + zck_free(&old_zck); if(!zck_dl_set_zck(target->target->zck_dl, zck)) { g_set_error(err, LR_DOWNLOADER_ERROR, LRE_ZCK, "Unable to setup zchunk download context"); From ee0a5b6e83b358b8acb2f86178952d8629fa899e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 19 Jul 2024 14:20:33 +0200 Subject: [PATCH 4/4] Unify freeing of `target->zck_dl` to a single place Apart from these two places the `zck_dl` is leaking through other code paths. Handle it and stuff it contains in a single place. --- librepo/downloader.c | 4 ---- librepo/downloadtarget.c | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/librepo/downloader.c b/librepo/downloader.c index 7208a407..5b610632 100644 --- a/librepo/downloader.c +++ b/librepo/downloader.c @@ -1357,8 +1357,6 @@ check_zck(LrTarget *target, GError **err) if(cks_good == 1) { // All checksums good g_debug("%s: File is complete", __func__); - if(target->target->zck_dl) - zck_dl_free(&(target->target->zck_dl)); target->zck_state = LR_ZCK_DL_FINISHED; return TRUE; } @@ -1378,8 +1376,6 @@ check_zck(LrTarget *target, GError **err) } if(cks_good == 1) { // All checksums good - if(target->target->zck_dl) - zck_dl_free(&(target->target->zck_dl)); target->zck_state = LR_ZCK_DL_FINISHED; return TRUE; } diff --git a/librepo/downloadtarget.c b/librepo/downloadtarget.c index 6825aef3..43953775 100644 --- a/librepo/downloadtarget.c +++ b/librepo/downloadtarget.c @@ -134,6 +134,26 @@ lr_downloadtarget_free(LrDownloadTarget *target) if (!target) return; + #ifdef WITH_ZCHUNK + if (target->zck_dl) { + zckCtx *zck = zck_dl_get_zck(target->zck_dl); + if (zck) { + zck_free(&zck); + } + + zckRange *range = zck_dl_get_range(target->zck_dl); + if(range) { + zck_range_free(&range); + } + + zck_dl_free(&target->zck_dl); + } + #endif /* WITH_ZCHUNK */ + + if(target->range) { + free(target->range); + } + g_slist_free_full(target->checksums, (GDestroyNotify) lr_downloadtargetchecksum_free); g_string_chunk_free(target->chunk);