From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 16 Mar 2023 13:18:49 +0000 (+0100)
Subject:  auth: Prevent a race during the processing of SVC auto-hints
X-Git-Tag: dnsdist-1.8.0~6^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=467ad1f020e8c63b2d0a289410a14c708ace5915;p=thirdparty%2Fpdns.git

 auth: Prevent a race during the processing of SVC auto-hints

When `svc-autohints` is enabled, the content of SVCB and HTTPS records
is modified in `PacketHandler::doAdditionalProcessing()` to expand
the IPv4 and IPv6 with their actual values.
This causes an issue because the content of these records might be
shared between threads, via the record cache, and one thread could
be trying to read from the internal `std::set` while a second thread
is altering it, leading to a data race and possibly to memory corruption
and a crash.
This is correctly detected by TSAN:
```
WARNING: ThreadSanitizer: data race (pid=102795)
  Write of size 8 at 0x7b3400010350 by thread T33:
    #0 operator delete(void*) <null> (pdns_server+0x211b7c) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #1 std::__new_allocator<std::_Rb_tree_node<SvcParam>>::deallocate(std::_Rb_tree_node<SvcParam>*, unsigned long) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/new_allocator.h:158:2 (pdns_server+0x33fc78) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #2 std::allocator_traits<std::allocator<std::_Rb_tree_node<SvcParam>>>::deallocate(std::allocator<std::_Rb_tree_node<SvcParam>>&, std::_Rb_tree_node<SvcParam>*, unsigned long) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/alloc_traits.h:496:13 (pdns_server+0x33fc78)
    #3 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_put_node(std::_Rb_tree_node<SvcParam>*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:565:9 (pdns_server+0x33fc78)
    #4 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_drop_node(std::_Rb_tree_node<SvcParam>*) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:632:2 (pdns_server+0x33fc78)
    #5 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::_M_erase_aux(std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:2495:7 (pdns_server+0x33fc78)
    #6 std::_Rb_tree<SvcParam, SvcParam, std::_Identity<SvcParam>, std::less<SvcParam>, std::allocator<SvcParam>>::erase[abi:cxx11](std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_tree.h:1197:2 (pdns_server+0x33fc78)
    #7 std::set<SvcParam, std::less<SvcParam>, std::allocator<SvcParam>>::erase[abi:cxx11](std::_Rb_tree_const_iterator<SvcParam>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/stl_set.h:655:21 (pdns_server+0x33fc78)
    #8 SVCBBaseRecordContent::setHints(SvcParam::SvcParamKey const&, std::vector<ComboAddress, std::allocator<ComboAddress>> const&) /work/pdns/pdns/dnsrecords.cc:768:14 (pdns_server+0x33fc78)
    #9 PacketHandler::doAdditionalProcessing(DNSPacket&, std::unique_ptr<DNSPacket, std::default_delete<DNSPacket>>&) /work/pdns/pdns/packethandler.cc:565:16 (pdns_server+0x4ed330) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #10 PacketHandler::doQuestion(DNSPacket&) /work/pdns/pdns/packethandler.cc:1794:5 (pdns_server+0x4f79b4) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    PowerDNS#11 PacketHandler::question(DNSPacket&) /work/pdns/pdns/packethandler.cc:1175:10 (pdns_server+0x4f649a) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    PowerDNS#12 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::distribute(int) /work/pdns/pdns/./distributor.hh:220:14 (pdns_server+0x260f70) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    PowerDNS#13 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()::operator()() const /work/pdns/pdns/./distributor.hh:179:25 (pdns_server+0x260b31) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    PowerDNS#14 void std::__invoke_impl<void, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(std::__invoke_other, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:61:14 (pdns_server+0x260b31)
    PowerDNS#15 std::__invoke_result<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>::type std::__invoke<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:96:14 (pdns_server+0x260b31)
    PowerDNS#16 void std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:258:13 (pdns_server+0x260b31)
    PowerDNS#17 std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::operator()() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:265:11 (pdns_server+0x260b31)
    PowerDNS#18 std:thread:_State_impl<std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>>::_M_run() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:210:13 (pdns_server+0x260b31)
    PowerDNS#19 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xd72c2) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)

  Previous read of size 2 at 0x7b3400010350 by thread T39:
    #0 SvcParam::getKey() const /work/pdns/pdns/./svc-records.hh:80:12 (pdns_server+0x3721f3) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #1 GenericDNSPacketWriter<std::vector<unsigned char, std::allocator<unsigned char>>>::xfrSvcParamKeyVals(std::set<SvcParam, std::less<SvcParam>, std::allocator<SvcParam>> const&) /work/pdns/pdns/dnswriter.cc:404:23 (pdns_server+0x3721f3)
    #2 void HTTPSRecordContent::xfrPacket<GenericDNSPacketWriter<std::vector<unsigned char, std::allocator<unsigned char>>>>(GenericDNSPacketWriter<std::vector<unsigned char, std::allocator<unsigned char>>>&, bool) /work/pdns/pdns/dnsrecords.cc:348:1 (pdns_server+0x3349bd) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #3 HTTPSRecordContent::toPacket(GenericDNSPacketWriter<std::vector<unsigned char, std::allocator<unsigned char>>>&) /work/pdns/pdns/dnsrecords.cc:348:1 (pdns_server+0x3349bd)
    #4 DNSRecordContent::serialize[abi:cxx11](DNSName const&, bool, bool) /work/pdns/pdns/./dnsparser.hh:215:11 (pdns_server+0x311140) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #5 DNSPacket::addRecord(DNSZoneRecord&&) /work/pdns/pdns/dnspacket.cc:177:68 (pdns_server+0x2fa894) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #6 PacketHandler::doAdditionalProcessing(DNSPacket&, std::unique_ptr<DNSPacket, std::default_delete<DNSPacket>>&) /work/pdns/pdns/packethandler.cc:542:8 (pdns_server+0x4eccf2) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #7 PacketHandler::doQuestion(DNSPacket&) /work/pdns/pdns/packethandler.cc:1794:5 (pdns_server+0x4f79b4) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #8 PacketHandler::question(DNSPacket&) /work/pdns/pdns/packethandler.cc:1175:10 (pdns_server+0x4f649a) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #9 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::distribute(int) /work/pdns/pdns/./distributor.hh:220:14 (pdns_server+0x260f70) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #10 MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()::operator()() const /work/pdns/pdns/./distributor.hh:179:25 (pdns_server+0x260b31) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    PowerDNS#11 void std::__invoke_impl<void, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(std::__invoke_other, MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:61:14 (pdns_server+0x260b31)
    PowerDNS#12 std::__invoke_result<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>::type std::__invoke<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>(MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:96:14 (pdns_server+0x260b31)
    PowerDNS#13 void std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::_M_invoke<0ul>(std::_Index_tuple<0ul>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:258:13 (pdns_server+0x260b31)
    PowerDNS#14 std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>::operator()() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:265:11 (pdns_server+0x260b31)
    PowerDNS#15 std:thread:_State_impl<std:thread:_Invoker<std::tuple<MultiThreadDistributor<DNSPacket, DNSPacket, PacketHandler>::MultiThreadDistributor(int)::'lambda'()>>>::_M_run() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:210:13 (pdns_server+0x260b31)
    PowerDNS#16 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xd72c2) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)

  Thread T33 'pdns/distributo' (tid=102833, running) created by thread T17 at:
    #0 pthread_create <null> (pdns_server+0x1904e6) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #1 __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663:35 (libstdc++.so.6+0xd73a9) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)
    #2 std:thread:_M_start_thread(std::unique_ptr<std:thread:_State, std::default_delete<std:thread:_State>>, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:147:37 (libstdc++.so.6+0xd73a9)
    #3 Distributor<DNSPacket, DNSPacket, PacketHandler>::Create(int) /work/pdns/pdns/./distributor.hh:134:18 (pdns_server+0x256d23) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #4 qthread(unsigned int) /work/pdns/pdns/auth-main.cc:536:25 (pdns_server+0x256d23)
    #5 void std::__invoke_impl<void, void (*)(unsigned int), unsigned int>(std::__invoke_other, void (*&&)(unsigned int), unsigned int&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:61:14 (pdns_server+0x2635f0) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #6 std::__invoke_result<void (*)(unsigned int), unsigned int>::type std::__invoke<void (*)(unsigned int), unsigned int>(void (*&&)(unsigned int), unsigned int&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:96:14 (pdns_server+0x2635f0)
    #7 void std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:258:13 (pdns_server+0x2635f0)
    #8 std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>::operator()() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:265:11 (pdns_server+0x2635f0)
    #9 std:thread:_State_impl<std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>>::_M_run() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:210:13 (pdns_server+0x2635f0)
    #10 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xd72c2) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)

  Thread T39 'pdns/distributo' (tid=102837, running) created by thread T19 at:
    #0 pthread_create <null> (pdns_server+0x1904e6) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #1 __gthread_create /usr/src/debug/gcc/gcc-build/x86_64-pc-linux-gnu/libstdc++-v3/include/x86_64-pc-linux-gnu/bits/gthr-default.h:663:35 (libstdc++.so.6+0xd73a9) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)
    #2 std:thread:_M_start_thread(std::unique_ptr<std:thread:_State, std::default_delete<std:thread:_State>>, void (*)()) /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:147:37 (libstdc++.so.6+0xd73a9)
    #3 Distributor<DNSPacket, DNSPacket, PacketHandler>::Create(int) /work/pdns/pdns/./distributor.hh:134:18 (pdns_server+0x256d23) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #4 qthread(unsigned int) /work/pdns/pdns/auth-main.cc:536:25 (pdns_server+0x256d23)
    #5 void std::__invoke_impl<void, void (*)(unsigned int), unsigned int>(std::__invoke_other, void (*&&)(unsigned int), unsigned int&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:61:14 (pdns_server+0x2635f0) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593)
    #6 std::__invoke_result<void (*)(unsigned int), unsigned int>::type std::__invoke<void (*)(unsigned int), unsigned int>(void (*&&)(unsigned int), unsigned int&&) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/invoke.h:96:14 (pdns_server+0x2635f0)
    #7 void std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:258:13 (pdns_server+0x2635f0)
    #8 std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>::operator()() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:265:11 (pdns_server+0x2635f0)
    #9 std:thread:_State_impl<std:thread:_Invoker<std::tuple<void (*)(unsigned int), unsigned int>>>::_M_run() /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/12.2.1/../../../../include/c++/12.2.1/bits/std_thread.h:210:13 (pdns_server+0x2635f0)
    #10 execute_native_thread_routine /usr/src/debug/gcc/gcc/libstdc++-v3/src/c++11/thread.cc:82:18 (libstdc++.so.6+0xd72c2) (BuildId: 6fe66a2d539a78c993bd2d377e00fad389220963)

SUMMARY: ThreadSanitizer: data race (/work/pdns-rgacogne/pdns/pdns_server+0x211b7c) (BuildId: 384adc19a67695435bd5e89d0a77f562561f4593) in operator delete(void*)
```

To prevent this issue, this commit clones the content of SVCB/HTTPS
records before modifying the copy. the drawback is that we need to
do this operation every single time we process them.
---

diff --git a/pdns/dnsrecords.cc b/pdns/dnsrecords.cc
index 475f12cfa5..e2212e36cd 100644
--- a/pdns/dnsrecords.cc
+++ b/pdns/dnsrecords.cc
@@ -341,7 +341,7 @@ boilerplate_conv(SVCB,
                  conv.xfr16BitInt(d_priority);
                  conv.xfrName(d_target, false, true);
                  if (d_priority != 0) {
-                   conv.xfrSvcParamKeyVals(*d_params.lock());
+                   conv.xfrSvcParamKeyVals(d_params);
                  }
                  )
 
@@ -349,7 +349,7 @@ boilerplate_conv(HTTPS,
                  conv.xfr16BitInt(d_priority);
                  conv.xfrName(d_target, false, true);
                  if (d_priority != 0) {
-                   conv.xfrSvcParamKeyVals(*d_params.lock());
+                   conv.xfrSvcParamKeyVals(d_params);
                  }
                  )
 
@@ -747,29 +747,28 @@ string APLRecordContent::getZoneRepresentation(bool /* noDot */) const {
 /* APL end */
 
 /* SVCB start */
-bool SVCBBaseRecordContent::autoHint(const SvcParam::SvcParamKey &key) {
-  auto params = d_params.lock();
-  auto p = getParamIt(key, *params);
-  if (p == params->end()) {
+bool SVCBBaseRecordContent::autoHint(const SvcParam::SvcParamKey &key) const {
+  auto p = getParamIt(key);
+  if (p == d_params.end()) {
     return false;
   }
   return p->getAutoHint();
 }
 
 void SVCBBaseRecordContent::setHints(const SvcParam::SvcParamKey &key, const std::vector<ComboAddress> &addresses) {
+  auto p = getParamIt(key);
+  if (p == d_params.end()) {
+    return;
+  }
+
   std::vector<ComboAddress> h;
   h.reserve(h.size() + addresses.size());
   h.insert(h.end(), addresses.begin(), addresses.end());
 
-  auto params = d_params.lock();
-  auto p = getParamIt(key, *params);
-  if (p == params->end()) {
-    return;
-  }
   try {
     auto newParam = SvcParam(key, std::move(h));
-    params->erase(p);
-    params->insert(newParam);
+    d_params.erase(p);
+    d_params.insert(newParam);
   } catch (...) {
     // XXX maybe we should SERVFAIL instead?
     return;
@@ -777,40 +776,47 @@ void SVCBBaseRecordContent::setHints(const SvcParam::SvcParamKey &key, const std
 }
 
 void SVCBBaseRecordContent::removeParam(const SvcParam::SvcParamKey &key) {
-  auto params = d_params.lock();
-  auto p = getParamIt(key, *params);
-  if (p == params->end()) {
+  auto p = getParamIt(key);
+  if (p == d_params.end()) {
     return;
   }
-  params->erase(p);
+  d_params.erase(p);
 }
 
-bool SVCBBaseRecordContent::hasParams() {
-  return !d_params.lock()->empty();
+bool SVCBBaseRecordContent::hasParams() const {
+  return !d_params.empty();
 }
 
-bool SVCBBaseRecordContent::hasParam(const SvcParam::SvcParamKey &key) {
-  auto params = d_params.lock();
-  return getParamIt(key, *params) != params->end();
+bool SVCBBaseRecordContent::hasParam(const SvcParam::SvcParamKey &key) const {
+  return getParamIt(key) != d_params.end();
 }
 
-SvcParam SVCBBaseRecordContent::getParam(const SvcParam::SvcParamKey &key) {
-  auto params = d_params.lock();
-  auto p = getParamIt(key, *params);
-  if (p == params->end()) {
+SvcParam SVCBBaseRecordContent::getParam(const SvcParam::SvcParamKey &key) const {
+  auto p = getParamIt(key);
+  if (p == d_params.end()) {
     throw std::out_of_range("No param with key " + SvcParam::keyToString(key));
   }
   return *p;
 }
 
-set<SvcParam>::const_iterator SVCBBaseRecordContent::getParamIt(const SvcParam::SvcParamKey &key, std::set<SvcParam>& params) {
-  auto p = std::find_if(params.begin(), params.end(),
+set<SvcParam>::const_iterator SVCBBaseRecordContent::getParamIt(const SvcParam::SvcParamKey &key) const {
+  auto p = std::find_if(d_params.begin(), d_params.end(),
       [&key](const SvcParam &param) {
         return param.getKey() == key;
       });
   return p;
 }
 
+std::shared_ptr<SVCBBaseRecordContent> SVCBRecordContent::clone() const
+{
+  return std::shared_ptr<SVCBBaseRecordContent>(std::make_shared<SVCBRecordContent>(*this));
+}
+
+std::shared_ptr<SVCBBaseRecordContent> HTTPSRecordContent::clone() const
+{
+  return std::shared_ptr<SVCBBaseRecordContent>(std::make_shared<HTTPSRecordContent>(*this));
+}
+
 /* SVCB end */
 
 boilerplate_conv(TKEY,
diff --git a/pdns/dnsrecords.hh b/pdns/dnsrecords.hh
index 245d8e8d4e..1d3b45c5a4 100644
--- a/pdns/dnsrecords.hh
+++ b/pdns/dnsrecords.hh
@@ -517,37 +517,40 @@ class SVCBBaseRecordContent : public DNSRecordContent
     const DNSName& getTarget() const {return d_target;}
     uint16_t getPriority() const {return d_priority;}
     // Returns true if a value for |key| was set to 'auto'
-    bool autoHint(const SvcParam::SvcParamKey &key);
+    bool autoHint(const SvcParam::SvcParamKey &key) const;
     // Sets the |addresses| to the existing hints for |key|
     void setHints(const SvcParam::SvcParamKey &key, const std::vector<ComboAddress> &addresses);
     // Removes the parameter for |key| from d_params
     void removeParam(const SvcParam::SvcParamKey &key);
     // Whether or not there are any parameter
-    bool hasParams();
+    bool hasParams() const;
     // Whether or not the param of |key| exists
-    bool hasParam(const SvcParam::SvcParamKey &key);
+    bool hasParam(const SvcParam::SvcParamKey &key) const;
     // Get the parameter with |key|, will throw out_of_range if param isn't there
-    SvcParam getParam(const SvcParam::SvcParamKey &key);
+    SvcParam getParam(const SvcParam::SvcParamKey &key) const;
+    virtual std::shared_ptr<SVCBBaseRecordContent> clone() const = 0;
 
   protected:
-    LockGuarded<set<SvcParam>> d_params;
+    std::set<SvcParam> d_params;
     DNSName d_target;
     uint16_t d_priority;
 
     // Get the iterator to parameter with |key|, return value can be d_params::end
-    static set<SvcParam>::const_iterator getParamIt(const SvcParam::SvcParamKey &key, set<SvcParam>& params);
+  std::set<SvcParam>::const_iterator getParamIt(const SvcParam::SvcParamKey &key) const;
 };
 
 class SVCBRecordContent : public SVCBBaseRecordContent
 {
 public:
   includeboilerplate(SVCB)
+  std::shared_ptr<SVCBBaseRecordContent> clone() const override;
 };
 
 class HTTPSRecordContent : public SVCBBaseRecordContent
 {
 public:
   includeboilerplate(HTTPS)
+  std::shared_ptr<SVCBBaseRecordContent> clone() const override;
 };
 
 class RRSIGRecordContent : public DNSRecordContent
diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index a1e3c71afe..81d157dcb6 100644
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -549,6 +549,12 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPack
     DNSName target = rrc->getTarget().isRoot() ? rec->dr.d_name : rrc->getTarget();
 
     if (rrc->hasParam(SvcParam::ipv4hint) && rrc->autoHint(SvcParam::ipv4hint)) {
+      auto newRRC = rrc->clone();
+      if (!newRRC) {
+        continue;
+      }
+      rrc = newRRC;
+      rec->dr.d_content = newRRC;
       if (s_SVCAutohints) {
         auto hints = getIPAddressFor(target, QType::A);
         if (hints.size() == 0) {
@@ -562,6 +568,12 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPack
     }
 
     if (rrc->hasParam(SvcParam::ipv6hint) && rrc->autoHint(SvcParam::ipv6hint)) {
+      auto newRRC = rrc->clone();
+      if (!newRRC) {
+        continue;
+      }
+      rrc = newRRC;
+      rec->dr.d_content = newRRC;
       if (s_SVCAutohints) {
         auto hints = getIPAddressFor(target, QType::AAAA);
         if (hints.size() == 0) {