When enabling sanitizers, gcc adds some instrumentation to the code that may enlarge stack frames. Some function's stack frames are already close to the limit of 4096 and are enlarged past that threshold, e.g. virLXCProcessStart which reaches a frame size of 4624 bytes. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 618cbd6b1d..bbdbe4afd8 100644 --- a/meson.build +++ b/meson.build @@ -278,7 +278,8 @@ cc_flags += [ '-Wformat-y2k', '-Wformat-zero-length', '-Wframe-address', - '-Wframe-larger-than=4096', + # sanitizer instrumentation may enlarge stack frames + &#39;-Wframe-larger-than=@0(a)&#39;.format(get_option(&#39;b_sanitize&#39;) in ['', 'none'] ? 4096 : 8192), '-Wfree-nonheap-object', '-Whsa', '-Wif-not-aligned', -- 2.26.3

When enabling sanitizers, clang adds some function symbols when instrumenting the code. The exact names of those functions are an implementation detail and should therefore not be added to any syms file. This patch prevents build failures due to those symbols not present in the syms file when building with sanitizers enabled. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- meson.build | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/meson.build b/meson.build index bbdbe4afd8..56c1294e7f 100644 --- a/meson.build +++ b/meson.build @@ -497,6 +497,11 @@ libvirt_no_indirect = cc.get_supported_link_arguments([ '-Wl,--no-copy-dt-needed-entries', ]) +if get_option('b_sanitize') not in [ '', 'none' ] + # sanitizers may add additional symbols + libvirt_no_undefined = [] +endif + if host_machine.system() == 'windows' version_script_flags = '-Wl,' else -- 2.26.3

When other preloaded libraries wrap and / or make calls to `realpath` (e.g. LLVM's AddessSanitizer), the second parameter is no longer guaranteed to be NULL. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- build-aux/syntax-check.mk | 2 +- tests/virfilemock.c | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 552d639119..03b7599abc 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1740,7 +1740,7 @@ exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \ exclude_file_name_regexp--sc_po_check = ^(docs/|src/rpc/gendispatch\.pl$$|tests/commandtest.c$$) exclude_file_name_regexp--sc_prohibit_PATH_MAX = \ - ^build-aux/syntax-check\.mk$$ + ^(build-aux/syntax-check\.mk|tests/virfilemock.c)$$ exclude_file_name_regexp--sc_prohibit_access_xok = \ ^(src/util/virutil\.c)$$ diff --git a/tests/virfilemock.c b/tests/virfilemock.c index 7c9174bdd9..cba66feac0 100644 --- a/tests/virfilemock.c +++ b/tests/virfilemock.c @@ -24,7 +24,6 @@ #if WITH_LINUX_MAGIC_H # include <linux/magic.h> #endif -#include <assert.h> #include "virmock.h" #include "virstring.h" @@ -186,15 +185,16 @@ realpath(const char *path, char *resolved) if (getenv("LIBVIRT_MTAB")) { const char *p; - char *ret; - assert(resolved == NULL); + if (!resolved) + resolved = g_new0(char, PATH_MAX); + if ((p = STRSKIP(path, "/some/symlink"))) - ret = g_strdup_printf("/gluster%s", p); + g_snprintf(resolved, PATH_MAX, "/gluster%s", p); else - ret = g_strdup(path); + g_strlcpy(resolved, path, PATH_MAX); - return ret; + return resolved; } return real_realpath(path, resolved); -- 2.26.3

"openvzutilstest" links, amongst others, against "libvirt_openvz.a" and "libvirt.so". The latter also links against "libvirt_openvz.a", leading to a One-Definition-Rule violation for "openvzLocateConfFile" in "openvz_conf.c". Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- src/libvirt_openvz.syms | 2 ++ tests/meson.build | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libvirt_openvz.syms b/src/libvirt_openvz.syms index ac0ed0d23e..a1038e51a4 100644 --- a/src/libvirt_openvz.syms +++ b/src/libvirt_openvz.syms @@ -3,10 +3,12 @@ # # openvz/openvz_conf.h +openvzCapsInit; openvzLocateConfFile; openvzReadConfigParam; openvzReadNetworkConf; openvzVEGetStringParam; +openvzXMLOption; # Let emacs know we want case-insensitive sorting # Local Variables: diff --git a/tests/meson.build b/tests/meson.build index 9900983d0c..3c73cbe3b5 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -430,7 +430,7 @@ endif if conf.has('WITH_OPENVZ') tests += [ - { 'name': 'openvzutilstest', 'link_with': [ openvz_lib ] }, + { 'name': 'openvzutilstest' }, ] endif -- 2.26.3

"virt-aa-helper" links, amongst others, against "datatypes.o" and "libvirt.so". The latter links against "libvirt_driver.a" which in turn also links against "datatypes.o", leading to a One-Definition-Rule violoation for "virConnectClass" et al. in "datatypes.c". Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- src/security/meson.build | 1 - 1 file changed, 1 deletion(-) diff --git a/src/security/meson.build b/src/security/meson.build index 416fec7900..6f5e1dec1d 100644 --- a/src/security/meson.build +++ b/src/security/meson.build @@ -41,7 +41,6 @@ if conf.has('WITH_LIBVIRTD') and conf.has('WITH_APPARMOR') 'name': 'virt-aa-helper', 'sources': [ apparmor_helper_sources, - datatypes_sources, dtrace_gen_objects, ], 'include': [ -- 2.26.3

meson supports the following sanitizers: "address" (e.g. out-of-bounds memory access, use-after-free, etc.), "thread" (data races), "undefined" (e.g. signed integer overflow), and "memory" (use of uninitialized memory). Note that not all sanitizers are supported by all compilers, and that more sanitizers exist. Not all sanitizers can be enabled at the same time, but "address" and "undefined" can. Both thread and memory sanitizers require an instrumented build of all dependencies, including libc. gcc and clang use different implementations of these sanitizers and have proven to find different issues. Create CI jobs for both. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- .gitlab-ci.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8b7df68f47..aa537e65fb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -71,6 +71,26 @@ stages: rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; fi +.sanitizer_build_job: + stage: builds + image: $CI_REGISTRY_IMAGE/ci-ubuntu-2004:latest + needs: + - x64-ubuntu-2004-container + rules: + - if: "$TEMPORARILY_DISABLED" + allow_failure: true + - when: on_success + cache: + paths: + - ccache/ + key: "$CI_JOB_NAME" + before_script: + - *script_variables + script: + - meson build --werror -Db_lundef=false -Db_sanitize="$SANITIZER" + - ninja -C build; + - ninja -C build test; + # Jobs that we delegate to Cirrus CI because they require an operating # system other than Linux. These jobs will only run if the required # setup has been performed on the GitLab account (see ci/README.rst). @@ -517,6 +537,21 @@ mingw64-fedora-rawhide: NAME: fedora-rawhide CROSS: mingw64 +# Sanitizers + +sanitize-gcc: + extends: .sanitizer_build_job + variables: + ASAN_OPTIONS: verify_asan_link_order=0 + CC: gcc + SANITIZER: address,undefined + +sanitize-clang: + extends: .sanitizer_build_job + variables: + CC: clang + SANITIZER: address,undefined + # This artifact published by this job is downloaded by libvirt.org to # be deployed to the web root: -- 2.26.3

On Mon, 2021-05-03 at 14:16 +0200, Pavel Hrdina wrote: I chose Ubuntu 20.04 for no particular reason other than "the image already has the sanitizers installed and need no change". Fedora 33 and Rawhide both require additional packages installed (libasan and libubsan). libvirt fails to build on Fedora Rawhide with gcc over what I believe to be a bug somewhere in the build chain. Output from one instance of the issue: ``` In file included from /usr/include/string.h:519, from ../source/src/internal.h:28, from ../source/src/rpc/virnettlscontext.h:23, from ../source/src/rpc/virnetclient.h:23, from ../source/src/rpc/virnetclient.c:27: In function ‘memcpy’, inlined from ‘virNetClientCallDispatchReply’ at ../source/src/rpc/virnetclient.c:1156:5, inlined from ‘virNetClientCallDispatch’ at ../source/src/rpc/virnetclient.c:1324:16, inlined from ‘virNetClientIOHandleInput’ at ../source/src/rpc/virnetclient.c:1497:23: /usr/include/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ offset [148, 167] from the object at ‘client’ is out of the bounds of referenced subobject ‘prog’ with type ‘unsigned int’ at offset 144 [- Werror=array-bounds] 29 | return __builtin___memcpy_chk (__dest, __src, __len, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 30 | __glibc_objsize0 (__dest)); | ~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from ../source/src/rpc/virnetmessage.h:23, from ../source/src/rpc/virnetclient.h:24, from ../source/src/rpc/virnetclient.c:27: ../source/src/rpc/virnetclient.c: In function ‘virNetClientIOHandleInput’: src/rpc/virnetprotocol.h:48:15: note: subobject ‘prog’ declared here 48 | u_int prog; | ^~~~ cc1: all warnings being treated as errors ``` Notice that source and destination of the memcpy referenced above are of type `virNetMessage`: ``` memcpy(&thecall->msg->header, &client->msg.header, sizeof(client- ``` Using clang, libvirt builds just fine on Fedora Rawhide, but half of the tests fail with: ``` ==58582==ERROR: AddressSanitizer failed to allocate 0x0 (0) bytes of SetAlternateSignalStack (error code: 22) (...) ==58582==AddressSanitizer CHECK failed: ../lib/sanitizer_common/sanitizer_common.cpp:54 "((0 && "unable to mmap")) != (0)" (0x0, 0x0) <empty stack> ``` On Fedora 33 (with libasan and libubsan installed manually), libvirt builds and tests fine with both clang and gcc, but find no additional issues in the code base. I cannot make a statement on the differences between the versions of (clang's) libasan and libubsan found in Ubuntu 20.04 and Fedora, but https://github.com/llvm/llvm-project/commits/main/compiler-rt/lib/asan and https://github.com/llvm/llvm-project/commits/main/compiler-rt/lib/ubsan/ suggest that not too many new features are added to sanitizers lately. Cheers, Tim