From 9bf2ae0e0a51d538a0faa95e55f8c1d93ae42b2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 11 Oct 2019 23:35:43 +0200 Subject: [PATCH 1/9] ci: Add LLVM/Clang scan-build checks into the GitLab CI (cherry picked from commit 5f584310bc139fb96fdf6aef523794ca8262ed32) --- .gitlab-ci.yml | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9bb5b133f0..7c466ea89d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -16,6 +16,8 @@ variables: TEST_PARALLEL_JOBS: 6 MAKE: make + CONFIGURE: ./configure + SCAN_BUILD: scan-build-9 stages: - precheck @@ -88,7 +90,7 @@ stages: .debian-buster-amd64: &debian_buster_amd64_image image: "$CI_REGISTRY_IMAGE:debian-buster-amd64" - <<: *linux_i386 + <<: *linux_amd64 .debian-sid-amd64: &debian_sid_amd64_image image: "$CI_REGISTRY_IMAGE:debian-sid-amd64" @@ -158,7 +160,7 @@ stages: expire_in: "1 week" .configure: &configure | - ./configure \ + ${CONFIGURE} \ --disable-maintainer-mode \ --enable-developer \ --with-libtool \ @@ -508,6 +510,38 @@ unit:gcc:buster:amd64: - gcc:buster:amd64 needs: ["gcc:buster:amd64"] +# Jobs for scan-build builds on Debian Buster (amd64) + +.scan_build: &scan_build | + ${SCAN_BUILD} --html-title="BIND 9 ($CI_COMMIT_SHORT_SHA)" \ + --keep-cc \ + --status-bugs \ + --keep-going \ + -o scan-build.reports \ + make -j${BUILD_PARALLEL_JOBS:-1} all V=1 + +scan-build:buster:amd64: + <<: *default_triggering_rules + <<: *debian_buster_amd64_image + stage: postcheck + variables: + CC: clang-9 + CFLAGS: "-Wall -Wextra -O2 -g" + CONFIGURE: "${SCAN_BUILD} ./configure" + EXTRA_CONFIGURE: "--enable-dnstap --with-libidn2" + script: + - *configure + - *scan_build + dependencies: + - autoreconf:sid:amd64 + needs: + - autoreconf:sid:amd64 + artifacts: + paths: + - scan-build.reports/ + expire_in: "1 week" + when: on_failure + # Jobs for regular GCC builds on Debian Sid (amd64) gcc:sid:amd64: From 1be81708885aa165006552e5f7e50c93c1d7f70d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sat, 12 Oct 2019 00:15:51 +0200 Subject: [PATCH 2/9] libdns: Remove useless checks for ISC_R_MEMORY, which cannot happen now (cherry picked from commit 80b55d25de1c116f2aad7c9585689f392e314ae7) --- lib/dns/client.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/lib/dns/client.c b/lib/dns/client.c index 172786b15d..85731df56e 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -933,21 +933,12 @@ client_resfind(resctx_t *rctx, dns_fetchevent_t *event) { * Otherwise, get some resource for copying the * result. */ - ansname = isc_mem_get(mctx, sizeof(*ansname)); - if (ansname == NULL) - tresult = ISC_R_NOMEMORY; - else { - dns_name_t *aname; + dns_name_t *aname = dns_fixedname_name(&rctx->name); - aname = dns_fixedname_name(&rctx->name); - dns_name_init(ansname, NULL); - tresult = dns_name_dup(aname, mctx, ansname); - if (tresult != ISC_R_SUCCESS) - isc_mem_put(mctx, ansname, - sizeof(*ansname)); - } - if (tresult != ISC_R_SUCCESS) - result = tresult; + ansname = isc_mem_get(mctx, sizeof(*ansname)); + dns_name_init(ansname, NULL); + + (void)dns_name_dup(aname, mctx, ansname); } switch (result) { From fcfdd847f41e3f330cf7ddffa95413e794d32cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 13 Oct 2019 06:40:25 +0200 Subject: [PATCH 3/9] tests: Workaround scan-build false positive with FD_ZERO/FD_SET (cherry picked from commit 7aa7f8592cf095712672070fdf5aec4e034d3a59) --- bin/tests/optional/zone_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/tests/optional/zone_test.c b/bin/tests/optional/zone_test.c index b26acea0b1..cf400f960c 100644 --- a/bin/tests/optional/zone_test.c +++ b/bin/tests/optional/zone_test.c @@ -149,12 +149,11 @@ query(void) { dns_fixedname_t name; dns_fixedname_t found; dns_db_t *db; - char *s; isc_buffer_t buffer; isc_result_t result; dns_rdataset_t rdataset; dns_rdataset_t sigset; - fd_set rfdset; + fd_set rfdset = { { 0 } }; db = NULL; result = dns_zone_getdb(zone, &db); @@ -169,7 +168,7 @@ query(void) { dns_rdataset_init(&sigset); do { - + char *s; fprintf(stdout, "zone_test "); fflush(stdout); FD_ZERO(&rfdset); From aaded0efe012a896fc6d375670b178d23eb51d2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 13 Oct 2019 06:47:26 +0200 Subject: [PATCH 4/9] named: Add INSIST() after bindkeysfile configuration load to silence scan-build FP (cherry picked from commit 6bf364aec87773764c2850a95251aa6a15cf320e) --- bin/named/server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/named/server.c b/bin/named/server.c index 148edb5bfc..5c72e36e9c 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8202,6 +8202,7 @@ load_configuration(const char *filename, named_server_t *server, INSIST(result == ISC_R_SUCCESS); CHECKM(setstring(server, &server->bindkeysfile, cfg_obj_asstring(obj)), "strdup"); + INSIST(server->bindkeysfile != NULL); if (access(server->bindkeysfile, R_OK) == 0) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, From 38866cb5c444d99596f487e459e1cf2ed26ec0d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 13 Oct 2019 06:53:06 +0200 Subject: [PATCH 5/9] dnssec: don't qsort() empty hashlist (cherry picked from commit 6bbb0b8e42cd7b2d6ffd9de8517f1a85e60c8019) --- bin/dnssec/dnssec-signzone.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 88b910227d..cce743913d 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -794,7 +794,10 @@ hashlist_comp(const void *a, const void *b) { static void hashlist_sort(hashlist_t *l) { - qsort(l->hashbuf, l->entries, l->length, hashlist_comp); + INSIST(l->hashbuf != NULL || l->length == 0); + if (l->length > 0) { + qsort(l->hashbuf, l->entries, l->length, hashlist_comp); + } } static bool From 7a0019cfa1506c1747dccc8784a89d6aa46acc23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 13 Oct 2019 07:02:34 +0200 Subject: [PATCH 6/9] tests: Resolve scan-build false positive by adding extra assertion (cherry picked from commit 309dca417cf4784c6453602aadd61bd9dd084878) --- bin/tests/system/dlzexternal/driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/tests/system/dlzexternal/driver.c b/bin/tests/system/dlzexternal/driver.c index 66fc069754..9c2c49457c 100644 --- a/bin/tests/system/dlzexternal/driver.c +++ b/bin/tests/system/dlzexternal/driver.c @@ -101,6 +101,7 @@ add_name(struct dlz_example_data *state, struct record *list, int first_empty = -1; for (i = 0; i < MAX_RECORDS; i++) { + INSIST(list[i].name != NULL); if (first_empty == -1 && strlen(list[i].name) == 0U) { first_empty = i; } From 72f9846be66bbfb7f9e4dd71c831a96128987f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 24 Oct 2019 13:55:56 +0200 Subject: [PATCH 7/9] libdns: Change check_dnskey_sigs() return type to void to match the reality how the function is used (cherry picked from commit 64cf5144a6873345877f9e18cca980474bf4e78d) --- lib/dns/zoneverify.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/dns/zoneverify.c b/lib/dns/zoneverify.c index b384f4ee9c..b83038d298 100644 --- a/lib/dns/zoneverify.c +++ b/lib/dns/zoneverify.c @@ -1521,7 +1521,7 @@ check_apex_rrsets(vctx_t *vctx) { * The variables to update are chosen based on 'is_ksk', which is true when * 'dnskey' is a KSK and false otherwise. */ -static isc_result_t +static void check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, dns_rdata_t *rdata, bool is_ksk) { @@ -1535,25 +1535,26 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, standby_keys = (is_ksk ? vctx->standby_ksk : vctx->standby_zsk); goodkey = (is_ksk ? &vctx->goodksk : &vctx->goodzsk); - if (dns_dnssec_selfsigns(rdata, vctx->origin, &vctx->keyset, + if (!dns_dnssec_selfsigns(rdata, vctx->origin, &vctx->keyset, &vctx->keysigs, false, vctx->mctx)) { - if (active_keys[dnskey->algorithm] != 255) { - active_keys[dnskey->algorithm]++; + if (!is_ksk && + dns_dnssec_signs(rdata, vctx->origin, &vctx->soaset, + &vctx->soasigs, false, vctx->mctx)) + { + if (active_keys[dnskey->algorithm] != 255) { + active_keys[dnskey->algorithm]++; + } + } else { + if (standby_keys[dnskey->algorithm] != 255) { + standby_keys[dnskey->algorithm]++; + } } - } else if (!is_ksk && - dns_dnssec_signs(rdata, vctx->origin, &vctx->soaset, - &vctx->soasigs, false, vctx->mctx)) - { - if (active_keys[dnskey->algorithm] != 255) { - active_keys[dnskey->algorithm]++; - } - return (ISC_R_SUCCESS); - } else { - if (standby_keys[dnskey->algorithm] != 255) { - standby_keys[dnskey->algorithm]++; - } - return (ISC_R_SUCCESS); + return; + } + + if (active_keys[dnskey->algorithm] != 255) { + active_keys[dnskey->algorithm]++; } /* @@ -1562,7 +1563,7 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, */ if (vctx->secroots == NULL) { *goodkey = true; - return (ISC_R_SUCCESS); + return; } /* @@ -1571,7 +1572,7 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, result = dns_dnssec_keyfromrdata(vctx->origin, rdata, vctx->mctx, &key); if (result != ISC_R_SUCCESS) { - return (result); + goto cleanup; } result = dns_keytable_findkeynode(vctx->secroots, vctx->origin, @@ -1582,10 +1583,6 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, * No such trust anchor. */ if (result != ISC_R_SUCCESS) { - if (result == DNS_R_PARTIALMATCH || result == ISC_R_NOTFOUND) { - result = ISC_R_SUCCESS; - } - goto cleanup; } @@ -1614,7 +1611,6 @@ check_dnskey_sigs(vctx_t *vctx, const dns_rdata_dnskey_t *dnskey, if (key != NULL) { dst_key_free(&key); } - return (ISC_R_SUCCESS); } /*% From 2d52a05f4f46081378639647b161ba17f7d0fdf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 31 Oct 2019 06:46:32 -0500 Subject: [PATCH 8/9] named: remove named_g_defaultdnstap global variable The named_g_defaultdnstap was never used as the dnstap requires explicit configuration of the output file. Related scan-build report: ./server.c:3476:14: warning: Value stored to 'dpath' during its initialization is never read const char *dpath = named_g_defaultdnstap; ^~~~~ ~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. (cherry picked from commit 6decd145926387347216f5a9ecbf8ca4593d11be) --- bin/named/include/named/globals.h | 8 -------- bin/named/server.c | 2 +- bin/named/win32/os.c | 1 - 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h index 49e75a1523..aa85453c98 100644 --- a/bin/named/include/named/globals.h +++ b/bin/named/include/named/globals.h @@ -135,14 +135,6 @@ EXTERN const char * named_g_defaultpidfile INIT(NAMED_LOCALSTATEDIR "/run/named.pid"); #endif -#ifdef HAVE_DNSTAP -EXTERN const char * named_g_defaultdnstap - INIT(NAMED_LOCALSTATEDIR "/run/named/" - "dnstap.sock"); -#else -EXTERN const char * named_g_defaultdnstap INIT(NULL); -#endif /* HAVE_DNSTAP */ - EXTERN const char * named_g_username INIT(NULL); EXTERN const char * named_g_engine INIT(NULL); diff --git a/bin/named/server.c b/bin/named/server.c index 5c72e36e9c..86d4eb8250 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -3485,7 +3485,7 @@ configure_dnstap(const cfg_obj_t **maps, dns_view_t *view) { isc_result_t result; const cfg_obj_t *obj, *obj2; const cfg_listelt_t *element; - const char *dpath = named_g_defaultdnstap; + const char *dpath; const cfg_obj_t *dlist = NULL; dns_dtmsgtype_t dttypes = 0; unsigned int i; diff --git a/bin/named/win32/os.c b/bin/named/win32/os.c index c1fe90410c..cb25ba0359 100644 --- a/bin/named/win32/os.c +++ b/bin/named/win32/os.c @@ -60,7 +60,6 @@ named_paths_init(void) { named_g_keyfile = isc_ntpaths_get(RNDC_KEY_PATH); named_g_defaultsessionkeyfile = isc_ntpaths_get(SESSION_KEY_PATH); named_g_defaultbindkeys = isc_ntpaths_get(BIND_KEYS_PATH); - named_g_defaultdnstap = NULL; Initialized = TRUE; } From 027f2c151846dc2dff17a0158d8d8b9986c2fbea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 31 Oct 2019 06:50:58 -0500 Subject: [PATCH 9/9] libdns: add missing checks for return values in dnstap unit test Related scan-build report: dnstap_test.c:169:2: warning: Value stored to 'result' is never read result = dns_test_makeview("test", &view); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ dnstap_test.c:193:2: warning: Value stored to 'result' is never read result = dns_compress_init(&cctx, -1, dt_mctx); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2 warnings generated. (cherry picked from commit e9acad638eb21e0ef0bd8558a196ca24c3099292) --- lib/dns/tests/dnstap_test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/dns/tests/dnstap_test.c b/lib/dns/tests/dnstap_test.c index e36d53eaac..93bef7267f 100644 --- a/lib/dns/tests/dnstap_test.c +++ b/lib/dns/tests/dnstap_test.c @@ -169,6 +169,7 @@ send_test(void **state) { cleanup(); result = dns_test_makeview("test", &view); + assert_int_equal(result, ISC_R_SUCCESS); fopt = fstrm_iothr_options_init(); assert_non_null(fopt); @@ -193,6 +194,7 @@ send_test(void **state) { memset(&zr, 0, sizeof(zr)); isc_buffer_init(&zb, zone, sizeof(zone)); result = dns_compress_init(&cctx, -1, mctx); + assert_int_equal(result, ISC_R_SUCCESS); dns_compress_setmethods(&cctx, DNS_COMPRESS_NONE); result = dns_name_towire(zname, &cctx, &zb); assert_int_equal(result, ISC_R_SUCCESS);