On Wed, Sep 20, 2023 at 05:05:06PM +0200, Stefano Brivio wrote:If we go over the flattened list of search domains and just replace dots and zero bytes with the length of the next label to implement the encoding specified by section 3.1 of RFC 1035, if there are multiple domains in the search list, we'll also replace separators between two domain names with the length of the first label of the second domain, plus one.That is... an impressively long sentence. Any chance you could reword that in shorter ones that are easier to follow ;).Those should remain as zero bytes to separate domains, though. To distinguish between label separators and domain names separators, for simplicity, introduce a dot before the first label of every domain we copy to form the list. All dots are then replaced by label lengths, and separators (zero bytes) remain as they are. As we do this, we need to make sure we don't replace the trailing dot, if present: that's already a separator. Skip copying it, and just add separators as needed. Now that we don't copy those, though, we might end up with zero-length domains: skip them, as they're meaningless anyway. And as we might skip domains, we can't use the index 'i' to check if we're at the beginning of the option -- use 'srch' instead. This is very similar to how we prepare the list for NDP option 31, except that we don't need padding (RFC 8106, 5.2) here, and we should refactor this into common functions, but it probably makes sense to rework the NDP responder (https://bugs.passt.top/show_bug.cgi?id=21) first. Reported-by: Sebastian Mitterle <smitterl(a)redhat.com> Link: https://bugs.passt.top/show_bug.cgi?id=75 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- dhcpv6.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/dhcpv6.c b/dhcpv6.c index fc42a84..58171bb 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -376,24 +376,34 @@ search: return offset; for (i = 0; *c->dns_search[i].n; i++) { - if (!i) { + size_t name_len = strlen(c->dns_search[i].n); + + /* We already append separators, don't duplicate if present */ + if (c->dns_search[i].n[name_len - 1] == '.') + name_len--; + + /* Skip root-only search domains */ + if (!name_len) + continue;Should we consider doing this normalisation when we build c->dns_search, rather than here?+ if (!srch) { srch = (struct opt_dns_search *)(buf + offset); offset += sizeof(struct opt_hdr); srch->hdr.t = OPT_DNS_SEARCH; srch->hdr.l = 0; p = srch->list; - *p = 0; } - p = stpcpy(p + 1, c->dns_search[i].n); - *(p++) = 0; - srch->hdr.l += strlen(c->dns_search[i].n) + 2; - offset += strlen(c->dns_search[i].n) + 2; + *p = '.'; + p = stpncpy(p + 1, c->dns_search[i].n, name_len); + p++; + srch->hdr.l += name_len + 2; + offset += name_len + 2; } if (srch) { for (i = 0; i < srch->hdr.l; i++) { - if (srch->list[i] == '.' || !srch->list[i]) { + if (srch->list[i] == '.') { srch->list[i] = strcspn(srch->list + i + 1, "."); }-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson