From c3a4f56818a937123279a4c0e3d785a1f71339f3 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Tue, 28 Feb 2017 18:24:40 +0000 Subject: [PATCH] Fix child-address counting. When a new address was created by a routing step it was possible for the parent address in the tree to be marked as having zero children, despite the new child having a pointer to the parent. When the child was then delivered, the count on the parent could go negative or, if other children had been added which correctly incremented the count, arrive at zero while some children were outstanding. Fix this to maintin the invariant. While there, make the counter unsigned. (cherry picked from commit 82f90600647a5322e9e7b58fc127eb8be839165c) Signed-off-by: Phil Pennock --- src/src/deliver.c | 16 ++++++++-------- src/src/routers/iplookup.c | 4 ++-- src/src/routers/queryprogram.c | 4 ++-- src/src/routers/redirect.c | 4 ++-- src/src/routers/rf_change_domain.c | 1 + src/src/structs.h | 2 +- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/src/deliver.c b/src/src/deliver.c index c97874a2b..0a1ea1990 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -671,7 +671,7 @@ address_item *aa; while (addr->parent) { addr = addr->parent; - if ((addr->child_count -= 1) > 0) return; /* Incomplete parent */ + if (--addr->child_count > 0) return; /* Incomplete parent */ address_done(addr, now); /* Log the completion of all descendents only when there is no ancestor with @@ -2975,13 +2975,13 @@ while (addr_local) addr3 = store_get(sizeof(address_item)); *addr3 = *addr2; addr3->next = NULL; - addr3->shadow_message = (uschar *) &(addr2->shadow_message); + addr3->shadow_message = US &addr2->shadow_message; addr3->transport = stp; addr3->transport_return = DEFER; addr3->return_filename = NULL; addr3->return_file = -1; *last = addr3; - last = &(addr3->next); + last = &addr3->next; } /* If we found any addresses to shadow, run the delivery, and stick any @@ -5031,6 +5031,7 @@ if (percent_hack_domains) address_item *new_parent = store_get(sizeof(address_item)); *new_parent = *addr; addr->parent = new_parent; + new_parent->child_count = 1; addr->address = new_address; addr->unique = string_copy(new_address); addr->domain = deliver_domain; @@ -5901,9 +5902,9 @@ else if (system_filter && process_recipients != RECIP_FAIL_TIMEOUT) while (p) { - if (parent->child_count == SHRT_MAX) + if (parent->child_count == USHRT_MAX) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "system filter generated more " - "than %d delivery addresses", SHRT_MAX); + "than %d delivery addresses", USHRT_MAX); parent->child_count++; p->parent = parent; @@ -7141,10 +7142,9 @@ for (addr_dsntmp = addr_succeed; addr_dsntmp; addr_dsntmp = addr_dsntmp->next) ) { /* copy and relink address_item and send report with all of them at once later */ - address_item *addr_next; - addr_next = addr_senddsn; + address_item * addr_next = addr_senddsn; addr_senddsn = store_get(sizeof(address_item)); - memcpy(addr_senddsn, addr_dsntmp, sizeof(address_item)); + *addr_senddsn = *addr_dsntmp; addr_senddsn->next = addr_next; } else diff --git a/src/src/routers/iplookup.c b/src/src/routers/iplookup.c index e6a35a7f3..96e9626df 100644 --- a/src/src/routers/iplookup.c +++ b/src/src/routers/iplookup.c @@ -382,9 +382,9 @@ new_addr->parent = addr; copyflag(new_addr, addr, af_propagate); new_addr->prop = addr->prop; -if (addr->child_count == SHRT_MAX) +if (addr->child_count == USHRT_MAX) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d " - "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address); + "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address); addr->child_count++; new_addr->next = *addr_new; *addr_new = new_addr; diff --git a/src/src/routers/queryprogram.c b/src/src/routers/queryprogram.c index bfcaefcfd..cd02f366f 100644 --- a/src/src/routers/queryprogram.c +++ b/src/src/routers/queryprogram.c @@ -120,9 +120,9 @@ while (generated != NULL) next->next = *addr_new; *addr_new = next; - if (addr->child_count == SHRT_MAX) + if (addr->child_count == USHRT_MAX) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d " - "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address); + "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address); addr->child_count++; DEBUG(D_route) diff --git a/src/src/routers/redirect.c b/src/src/routers/redirect.c index 8aad1d4ab..29537baae 100644 --- a/src/src/routers/redirect.c +++ b/src/src/routers/redirect.c @@ -335,9 +335,9 @@ while (generated) next->parent = addr; orflag(next, addr, af_ignore_error); next->start_router = rblock->redirect_router; - if (addr->child_count == SHRT_MAX) + if (addr->child_count == USHRT_MAX) log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d " - "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address); + "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address); addr->child_count++; next->next = *addr_new; diff --git a/src/src/routers/rf_change_domain.c b/src/src/routers/rf_change_domain.c index 8986925c1..219e283cc 100644 --- a/src/src/routers/rf_change_domain.c +++ b/src/src/routers/rf_change_domain.c @@ -57,6 +57,7 @@ addr->prop = parent->prop; addr->address = address; addr->unique = string_copy(address); addr->parent = parent; +parent->child_count = 1; addr->next = *addr_new; *addr_new = addr; diff --git a/src/src/structs.h b/src/src/structs.h index d9d37f1c0..38b095f06 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -625,7 +625,7 @@ typedef struct address_item { /* (may need to hold a timestamp) */ short int basic_errno; /* status after failure */ - short int child_count; /* number of child addresses */ + unsigned short child_count; /* number of child addresses */ short int return_file; /* fileno of return data file */ short int special_action; /* ( used when when deferred or failed */ /* ( also */ -- 2.30.2