When replacing cache entries, preserve CNAMES which target them.

When inserting new cache records, we first delete existing records
of the same name/type, to maintain consistency. This has the side effect
of deleting any CNAMES which have the records as target. So

cname1.example CNAME record.example
cname2.example CNAME record.example

looking up cname2.example will push it into the cache, and also
push record.example. Doing that deletes any cache of cname1.example.

This changeset avoids that problem by making sure that when
deleting record.example, and re-insterting it (with the same name -important),
it uses the same struct crec, with the same uid. This preserves the existing cnames.
This commit is contained in:
Simon Kelley
2018-07-18 20:59:52 +01:00
parent 45d8a2435e
commit eb1fe15ca8

View File

@@ -325,7 +325,8 @@ static int is_expired(time_t now, struct crec *crecp)
return 1; return 1;
} }
static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t now, unsigned short flags) static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t now, unsigned short flags,
struct crec **target_crec, unsigned int *target_uid)
{ {
/* Scan and remove old entries. /* Scan and remove old entries.
If (flags & F_FORWARD) then remove any forward entries for name and any expired If (flags & F_FORWARD) then remove any forward entries for name and any expired
@@ -338,7 +339,10 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
to a cache entry if the name exists in the cache as a HOSTS or DHCP entry (these are never deleted) to a cache entry if the name exists in the cache as a HOSTS or DHCP entry (these are never deleted)
We take advantage of the fact that hash chains have stuff in the order <reverse>,<other>,<immortal> We take advantage of the fact that hash chains have stuff in the order <reverse>,<other>,<immortal>
so that when we hit an entry which isn't reverse and is immortal, we're done. */ so that when we hit an entry which isn't reverse and is immortal, we're done.
If we free a crec which is a CNAME target, return the entry and uid in target_crec and target_uid.
This entry will get re-used with the same name, to preserve CNAMEs. */
struct crec *crecp, **up; struct crec *crecp, **up;
@@ -346,17 +350,6 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
{ {
for (up = hash_bucket(name), crecp = *up; crecp; crecp = crecp->hash_next) for (up = hash_bucket(name), crecp = *up; crecp; crecp = crecp->hash_next)
{ {
if (is_expired(now, crecp) || is_outdated_cname_pointer(crecp))
{
*up = crecp->hash_next;
if (!(crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)))
{
cache_unlink(crecp);
cache_free(crecp);
}
continue;
}
if ((crecp->flags & F_FORWARD) && hostname_isequal(cache_get_name(crecp), name)) if ((crecp->flags & F_FORWARD) && hostname_isequal(cache_get_name(crecp), name))
{ {
/* Don't delete DNSSEC in favour of a CNAME, they can co-exist */ /* Don't delete DNSSEC in favour of a CNAME, they can co-exist */
@@ -366,6 +359,16 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
if (crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) if (crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG))
return crecp; return crecp;
*up = crecp->hash_next; *up = crecp->hash_next;
/* If this record is for the name we're inserting and is the target
of a CNAME record. Make the new record for the same name, in the same
crec, with the same uid to avoid breaking the existing CNAME. */
if (crecp->uid != UID_NONE)
{
if (target_crec)
*target_crec = crecp;
if (target_uid)
*target_uid = crecp->uid;
}
cache_unlink(crecp); cache_unlink(crecp);
cache_free(crecp); cache_free(crecp);
continue; continue;
@@ -384,6 +387,18 @@ static struct crec *cache_scan_free(char *name, struct all_addr *addr, time_t no
} }
#endif #endif
} }
if (is_expired(now, crecp) || is_outdated_cname_pointer(crecp))
{
*up = crecp->hash_next;
if (!(crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)))
{
cache_unlink(crecp);
cache_free(crecp);
}
continue;
}
up = &crecp->hash_next; up = &crecp->hash_next;
} }
} }
@@ -450,10 +465,11 @@ void cache_start_insert(void)
struct crec *cache_insert(char *name, struct all_addr *addr, struct crec *cache_insert(char *name, struct all_addr *addr,
time_t now, unsigned long ttl, unsigned short flags) time_t now, unsigned long ttl, unsigned short flags)
{ {
struct crec *new; struct crec *new, *target_crec = NULL;
union bigname *big_name = NULL; union bigname *big_name = NULL;
int freed_all = flags & F_REVERSE; int freed_all = flags & F_REVERSE;
int free_avail = 0; int free_avail = 0;
unsigned int target_uid;
/* Don't log DNSSEC records here, done elsewhere */ /* Don't log DNSSEC records here, done elsewhere */
if (flags & (F_IPV4 | F_IPV6 | F_CNAME)) if (flags & (F_IPV4 | F_IPV6 | F_CNAME))
@@ -472,7 +488,7 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
/* First remove any expired entries and entries for the name/address we /* First remove any expired entries and entries for the name/address we
are currently inserting. */ are currently inserting. */
if ((new = cache_scan_free(name, addr, now, flags))) if ((new = cache_scan_free(name, addr, now, flags, &target_crec, &target_uid)))
{ {
/* We're trying to insert a record over one from /* We're trying to insert a record over one from
/etc/hosts or DHCP, or other config. If the /etc/hosts or DHCP, or other config. If the
@@ -496,6 +512,7 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
} }
/* Now get a cache entry from the end of the LRU list */ /* Now get a cache entry from the end of the LRU list */
if (!target_crec)
while (1) { while (1) {
if (!(new = cache_tail)) /* no entries left - cache is too small, bail */ if (!(new = cache_tail)) /* no entries left - cache is too small, bail */
{ {
@@ -503,12 +520,14 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
return NULL; return NULL;
} }
/* Free entry at end of LRU list, use it. */
if (!(new->flags & (F_FORWARD | F_REVERSE)))
break;
/* End of LRU list is still in use: if we didn't scan all the hash /* End of LRU list is still in use: if we didn't scan all the hash
chains for expired entries do that now. If we already tried that chains for expired entries do that now. If we already tried that
then it's time to start spilling things. */ then it's time to start spilling things. */
if (new->flags & (F_FORWARD | F_REVERSE))
{
/* If free_avail set, we believe that an entry has been freed. /* If free_avail set, we believe that an entry has been freed.
Bugs have been known to make this not true, resulting in Bugs have been known to make this not true, resulting in
a tight loop here. If that happens, abandon the a tight loop here. If that happens, abandon the
@@ -536,15 +555,14 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
#endif #endif
free_avail = 1; /* Must be free space now. */ free_avail = 1; /* Must be free space now. */
cache_scan_free(cache_get_name(new), &free_addr, now, new->flags); cache_scan_free(cache_get_name(new), &free_addr, now, new->flags, NULL, NULL);
cache_live_freed++; cache_live_freed++;
} }
else else
{ {
cache_scan_free(NULL, NULL, now, 0); cache_scan_free(NULL, NULL, now, 0, NULL, NULL);
freed_all = 1; freed_all = 1;
} }
continue;
} }
/* Check if we need to and can allocate extra memory for a long name. /* Check if we need to and can allocate extra memory for a long name.
@@ -567,10 +585,16 @@ struct crec *cache_insert(char *name, struct all_addr *addr,
} }
/* If we freed a cache entry for our name which was a CNAME target, use that.
and preserve the uid, so that existing CNAMES are not broken. */
if (target_crec)
{
new = target_crec;
new->uid = target_uid;
}
/* Got the rest: finally grab entry. */ /* Got the rest: finally grab entry. */
cache_unlink(new); cache_unlink(new);
break;
}
new->flags = flags; new->flags = flags;
if (big_name) if (big_name)
@@ -1248,7 +1272,7 @@ void cache_add_dhcp_entry(char *host_name, int prot,
} }
else if (!(crec->flags & F_DHCP)) else if (!(crec->flags & F_DHCP))
{ {
cache_scan_free(host_name, NULL, 0, crec->flags & (flags | F_CNAME | F_FORWARD)); cache_scan_free(host_name, NULL, 0, crec->flags & (flags | F_CNAME | F_FORWARD), NULL, NULL);
/* scan_free deletes all addresses associated with name */ /* scan_free deletes all addresses associated with name */
break; break;
} }
@@ -1275,7 +1299,7 @@ void cache_add_dhcp_entry(char *host_name, int prot,
if (crec->flags & F_NEG) if (crec->flags & F_NEG)
{ {
flags |= F_REVERSE; flags |= F_REVERSE;
cache_scan_free(NULL, (struct all_addr *)host_address, 0, flags); cache_scan_free(NULL, (struct all_addr *)host_address, 0, flags, NULL, NULL);
} }
} }
else else