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 <thomas@corelatus.se>
This commit is contained in:
Miroslav Lichvar 2023-09-11 15:29:04 +02:00
parent 43320a1d6b
commit 7ff74d9efe
2 changed files with 44 additions and 34 deletions

57
conf.c
View file

@ -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];
}
}

View file

@ -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