From: Miroslav Lichvar Date: Mon, 11 Sep 2023 13:29:04 +0000 (+0200) Subject: conf: fix reloading modified sources specified by IP address X-Git-Tag: 4.5-pre1~36 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7ff74d9efe1cd81160282c90d1f75d35ae3ba521;p=thirdparty%2Fchrony.git conf: fix reloading modified sources specified by IP address When reloading a modified source from sourcedir which is ordered before the original source (e.g. maxpoll was decreased), the new source is added before the original one is removed. If the source is specified by IP address, the addition fails due to the conflict with the original source. Sources specified by hostname don't conflict. They are resolved later (repeatedly if the resolver provides only conflicting addresses). Split the processing of sorted source lists into two phases, so all modified sources are removed before they are added again to avoid the conflict. Reported-by: Thomas Lange --- diff --git a/conf.c b/conf.c index f9840609..4c1d2e90 100644 --- a/conf.c +++ b/conf.c @@ -1679,7 +1679,7 @@ reload_source_dirs(void) uint32_t *prev_ids, *new_ids; char buf[MAX_LINE_LENGTH]; NSR_Status s; - int d; + int d, pass; prev_size = ARR_GetSize(ntp_source_ids); if (prev_size > 0 && ARR_GetSize(ntp_sources) != prev_size) @@ -1713,36 +1713,37 @@ reload_source_dirs(void) qsort(new_sources, new_size, sizeof (new_sources[0]), compare_sources); - for (i = j = 0; i < prev_size || j < new_size; ) { - if (i < prev_size && j < new_size) - d = compare_sources(&prev_sources[i], &new_sources[j]); - else - d = i < prev_size ? -1 : 1; + for (pass = 0; pass < 2; pass++) { + for (i = j = 0; i < prev_size || j < new_size; i += d <= 0, j += d >= 0) { + if (i < prev_size && j < new_size) + d = compare_sources(&prev_sources[i], &new_sources[j]); + else + d = i < prev_size ? -1 : 1; - if (d < 0) { - /* Remove the missing source */ - if (prev_sources[i].params.name[0] != '\0') + /* Remove missing sources before adding others to avoid conflicts */ + if (pass == 0 && d < 0 && prev_sources[i].params.name[0] != '\0') { NSR_RemoveSourcesById(prev_ids[i]); - i++; - } else if (d > 0) { - /* Add a newly configured source */ - source = &new_sources[j]; - s = NSR_AddSourceByName(source->params.name, source->params.port, source->pool, - source->type, &source->params.params, &new_ids[j]); - - if (s == NSR_UnresolvedName) { - unresolved++; - } else if (s != NSR_Success) { - LOG(LOGS_ERR, "Could not add source %s", source->params.name); - - /* Mark the source as not present */ - source->params.name[0] = '\0'; } - j++; - } else { - /* Keep the existing source */ - new_ids[j] = prev_ids[i]; - i++, j++; + + /* Add new sources */ + if (pass == 1 && d > 0) { + source = &new_sources[j]; + s = NSR_AddSourceByName(source->params.name, source->params.port, source->pool, + source->type, &source->params.params, &new_ids[j]); + + if (s == NSR_UnresolvedName) { + unresolved++; + } else if (s != NSR_Success) { + LOG(LOGS_ERR, "Could not add source %s", source->params.name); + + /* Mark the source as not present */ + source->params.name[0] = '\0'; + } + } + + /* Keep unchanged sources */ + if (pass == 1 && d == 0) + new_ids[j] = prev_ids[i]; } } diff --git a/test/system/008-confload b/test/system/008-confload index a7fc1d52..7e806988 100755 --- a/test/system/008-confload +++ b/test/system/008-confload @@ -30,6 +30,8 @@ echo "server 127.123.4.4" > $TEST_DIR/conf4.d/4.conf echo "server 127.123.5.1" > $TEST_DIR/conf5.d/1.sources echo "server 127.123.5.2" > $TEST_DIR/conf5.d/2.sources echo "server 127.123.5.3" > $TEST_DIR/conf5.d/3.sources +echo "server 127.123.5.4" > $TEST_DIR/conf5.d/4.sources +echo "server 127.123.5.5" > $TEST_DIR/conf5.d/5.sources start_chronyd || test_fail @@ -46,12 +48,16 @@ check_chronyc_output "^[^=]* .. 127\.123\.1\.2 [^^]* .. 127\.123\.5\.1 [^^]* .. 127\.123\.5\.2 [^^]* -.. 127\.123\.5\.3 [^^]*$" || test_fail +.. 127\.123\.5\.3 [^^]* +.. 127\.123\.5\.4 [^^]* +.. 127\.123\.5\.5 [^^]*$" || test_fail rm $TEST_DIR/conf5.d/1.sources -echo "server 127.123.5.2 minpoll 7" > $TEST_DIR/conf5.d/2.sources -echo > $TEST_DIR/conf5.d/3.sources -echo "server 127.123.5.4" > $TEST_DIR/conf5.d/4.sources +echo "server 127.123.5.2 minpoll 5" > $TEST_DIR/conf5.d/2.sources +echo "server 127.123.5.3 minpoll 7" > $TEST_DIR/conf5.d/3.sources +echo > $TEST_DIR/conf5.d/4.sources +echo "server 127.123.5.5" >> $TEST_DIR/conf5.d/5.sources +echo "server 127.123.5.6" > $TEST_DIR/conf5.d/6.sources run_chronyc "reload sources" || test_fail @@ -66,9 +72,12 @@ check_chronyc_output "^[^=]* .. 127\.123\.2\.3 [^^]* .. 127\.123\.4\.4 [^^]* .. 127\.123\.1\.2 *[05] 6 [^^]* -.. 127\.123\.5\.2 *[05] 7 [^^]* -.. 127\.123\.5\.4 [^^]*$" || test_fail +.. 127\.123\.5\.5 [^^]* +.. 127\.123\.5\.2 *[05] 5 [^^]* +.. 127\.123\.5\.3 *[05] 7 [^^]* +.. 127\.123\.5\.6 [^^]*$" || test_fail stop_chronyd || test_fail +check_chronyd_message_count "Could not add source" 1 1 || test_fail test_pass