From ae45e1a3832fbb6c96707687e42f0b4aaab52c9b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 29 Dec 2020 23:50:54 +0900 Subject: [PATCH] resolve: slightly optimize dns_answer_add() Previously, dns_answer_add() was O(n^2). With this change dns_packet_extract() becomes ~15 times faster for some extremal case. Before: ``` $ time ./fuzz-dns-packet ~/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808 /home/watanabe/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808... ok real 0m15.453s user 0m15.430s sys 0m0.007s ``` After: ``` $ time ./fuzz-dns-packet ~/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808 /home/watanabe/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808... ok real 0m0.831s user 0m0.824s sys 0m0.006s ``` Hopefully fixes oss-fuzz#19227. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19227 --- src/resolve/resolved-dns-answer.c | 89 ++++++++++++++++------- src/resolve/resolved-dns-answer.h | 2 + src/resolve/resolved-dns-rr.c | 2 +- src/resolve/resolved-dns-rr.h | 1 + test/fuzz/fuzz-dns-packet/oss-fuzz-19227 | Bin 0 -> 30586 bytes 5 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 test/fuzz/fuzz-dns-packet/oss-fuzz-19227 diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 5b762a82e8..f5e50fcd84 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -8,18 +8,53 @@ #include "resolved-dns-dnssec.h" #include "string-util.h" +static void dns_answer_item_hash_func(const DnsAnswerItem *a, struct siphash *state) { + assert(a); + assert(state); + + siphash24_compress(&a->ifindex, sizeof(a->ifindex), state); + + dns_resource_record_hash_func(a->rr, state); +} + +static int dns_answer_item_compare_func(const DnsAnswerItem *a, const DnsAnswerItem *b) { + int r; + + assert(a); + assert(b); + + r = CMP(a->ifindex, b->ifindex); + if (r != 0) + return r; + + return dns_resource_record_compare_func(a->rr, b->rr); +} + +DEFINE_PRIVATE_HASH_OPS(dns_answer_item_hash_ops, DnsAnswerItem, dns_answer_item_hash_func, dns_answer_item_compare_func); + DnsAnswer *dns_answer_new(size_t n) { + _cleanup_set_free_ Set *s = NULL; DnsAnswer *a; if (n > UINT16_MAX) /* We can only place 64K RRs in an answer at max */ n = UINT16_MAX; + s = set_new(&dns_answer_item_hash_ops); + if (!s) + return NULL; + + /* Higher multipliers give slightly higher efficiency through hash collisions, but the gains + * quickly drop off after 2. */ + if (set_reserve(s, n * 2) < 0) + return NULL; + a = malloc0(offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * n); if (!a) return NULL; a->n_ref = 1; a->n_allocated = n; + a->set_items = TAKE_PTR(s); return a; } @@ -30,6 +65,8 @@ static void dns_answer_flush(DnsAnswer *a) { if (!a) return; + a->set_items = set_free(a->set_items); + DNS_ANSWER_FOREACH(rr, a) dns_resource_record_unref(rr); @@ -46,6 +83,8 @@ static DnsAnswer *dns_answer_free(DnsAnswer *a) { DEFINE_TRIVIAL_REF_UNREF_FUNC(DnsAnswer, dns_answer, dns_answer_free); static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { + int r; + assert(rr); if (!a) @@ -54,12 +93,21 @@ static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, if (a->n_rrs >= a->n_allocated) return -ENOSPC; - a->items[a->n_rrs++] = (DnsAnswerItem) { - .rr = dns_resource_record_ref(rr), + a->items[a->n_rrs] = (DnsAnswerItem) { + .rr = rr, .ifindex = ifindex, .flags = flags, }; + r = set_put(a->set_items, &a->items[a->n_rrs]); + if (r < 0) + return r; + if (r == 0) + return -EEXIST; + + dns_resource_record_ref(rr); + a->n_rrs++; + return 1; } @@ -78,8 +126,7 @@ static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) { } int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { - size_t i; - int r; + DnsAnswerItem tmp, *exist; assert(rr); @@ -88,36 +135,26 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFl if (a->n_ref > 1) return -EBUSY; - for (i = 0; i < a->n_rrs; i++) { - if (a->items[i].ifindex != ifindex) - continue; - - r = dns_resource_key_equal(a->items[i].rr->key, rr->key); - if (r < 0) - return r; - if (r == 0) - continue; + tmp = (DnsAnswerItem) { + .rr = rr, + .ifindex = ifindex, + }; + exist = set_get(a->set_items, &tmp); + if (exist) { /* There's already an RR of the same RRset in place! Let's see if the TTLs more or less * match. We don't really care if they match precisely, but we do care whether one is 0 and * the other is not. See RFC 2181, Section 5.2. */ - if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0)) + if ((rr->ttl == 0) != (exist->rr->ttl == 0)) return -EINVAL; - r = dns_resource_record_payload_equal(a->items[i].rr, rr); - if (r < 0) - return r; - if (r == 0) - continue; - - /* Entry already exists, keep the entry with the higher RR. */ - if (rr->ttl > a->items[i].rr->ttl) { - dns_resource_record_ref(rr); - dns_resource_record_unref(a->items[i].rr); - a->items[i].rr = rr; + /* Entry already exists, keep the entry with the higher TTL. */ + if (rr->ttl > exist->rr->ttl) { + dns_resource_record_unref(exist->rr); + exist->rr = dns_resource_record_ref(rr); } - a->items[i].flags |= flags; + exist->flags |= flags; return 0; } diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index fd94c516de..d73525cedd 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -6,6 +6,7 @@ typedef struct DnsAnswerItem DnsAnswerItem; #include "macro.h" #include "resolved-dns-rr.h" +#include "set.h" /* A simple array of resource records. We keep track of the * originating ifindex for each RR where that makes sense, so that we @@ -30,6 +31,7 @@ struct DnsAnswerItem { struct DnsAnswer { unsigned n_ref; + Set *set_items; /* Used by dns_answer_add() for optimization. */ size_t n_rrs, n_allocated; DnsAnswerItem items[0]; }; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index c848da3ccb..19e075479f 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1479,7 +1479,7 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash * } } -static int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) { +int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) { int r; r = dns_resource_key_compare_func(x->key, y->key); diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 59b3a70179..aa17d6d391 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -330,6 +330,7 @@ DnsTxtItem *dns_txt_item_copy(DnsTxtItem *i); int dns_txt_item_new_empty(DnsTxtItem **ret); void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state); +int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y); extern const struct hash_ops dns_resource_key_hash_ops; extern const struct hash_ops dns_resource_record_hash_ops; diff --git a/test/fuzz/fuzz-dns-packet/oss-fuzz-19227 b/test/fuzz/fuzz-dns-packet/oss-fuzz-19227 new file mode 100644 index 0000000000000000000000000000000000000000..62828e7446a07cfe8e449629be937afb636ac920 GIT binary patch literal 30586 zcmeHQTZ|<~d9E`v>s{}fcmc=QfXz5yL$KZM`(+o$JC_}zM0v8R20%ZjO3*bFiNT)N7BG z$>XJd#H=4Lv&T#Qh(+Nd_v-1)|68nUtbVlXubz>;3BHe)+2f^t6Wu#X$nkdOc&Q(y z9B!S~$?^5;w@&31qp);yEd{SohUMbsr$m$dSBbb?!|@Y*{gs+C+~j27CSgY=@SpPT zADJ8_q2qN?$4g!I=9bPJ3A0B+$I?0iz6 z9vn-R>AY^me^B9gfTb*T8$Jw<>SS&w2H_hH(!<60f zT#|>QGG0$cHq3gtkz`f4Zo!oMP3awsg4&9?d~mqGyYr+4GYpD293~?nVuRd9WEww`RD$gA@2Zz-tE*Fm}R4`iigEB4_%Mh5PEJjICH4FIxCJ&;< zPpZ&{xtMHaYolPX{(#O_H42L$9NOrVVUdHS+={6WyfRNCE9S{&=P>cD@|h}-u+eth z!E@E@51aif2e!8AY!s~*#g+~8_q0ACz<5Fy<$9V|Wf0pKAC;qQIIQAm5hWjH26Ih^aUB+mK(xiZ zD?@KsZ4`lUL@nbGc(TZQnxKt1&5FEE7c&k@8(FrtmIkp1944i5v#QpcxUz#r8C+4~ z3#;u=+HdMS%_HH`yPyR=-2_#OoUsXNf^rxo!!TW_nGnIKys~}RAZXb@PnFws9yY;Z z%>8=Z|m{-rlef-oij>E!f;D ztJsDr;RX>#%i0$mG-#FEm!59Co!!hAgKVHCW|hZbBs{Nyda^lu$~$;I^g|I(4b;s2 z1htL(gM+8MtvD|IK&Y`n3l8zJmz1NX3Hs{Lq9FRk?#x6j_Gc!lXtJWrq99nzJzBiz z*I`o+{c5pPf!eBkZ#M|SLMV>eB6NBG($01{6p_e8E%u=%3Qn}ChIJTNTU_i#OV``}p76+d11%BZP=h}fK zk(Xs&=*h`R2bOzb?4`Mv2>s~75-$n8qVR;9>cDV4^&)g(u|U{`BJeUV@dBAjwbWpE zKD<5>|19x^r!rv{M{B|`^GSxya8zf)Cz!Che9k*aQz+8np$Bgb61eITLJG3|-Xznc zrl%#tx=tJ6AWayePM$;&>{L*D^P5`Bn#g11M`4tUVK8t3@`@4>#Y1!z)?nanxYD!` zHfZ41GCyg`WMd5ppb(0I+raEISgY3RD%=nw*1^G8>ujw^;x(al2JT|G6|ANCR+evV z3BP3ECOKzT*I`}S{ugt?$yy6OT`0znME z%G!^RxCjo}As2KMF0BAbdq2WB^#Zu$&>@$|i@g9n2_5~2*j=bgASWapxuC(+%R(>o z7uy@<3`!An3L>=Y3jh>f=E-?ir(L*sXhMvCc74tOghoUC?D_&6f+RYU+VviRFNU%p z%7{#*!yf__I*W0O?YIJ?hIYZFy&u38kaFhbb~z(<7hWC$y%>ieV@svlegMZ@YE%rH z(a<7i;HpapRhZXF6(o!ODRA|=Kgz>24+@!dneEPk;JnPn#LXrXSsIDhd*7Se_!e%Y z@o?Q=58`U`q79h&o3*Et_Yw(p-+~Q_M&!yXVVVYTR%!AshNDpUKm+*xW^cRMXJVK( zwNQo@NMk~h7Q=TiAzIwD_HOiRaQ#qNJH^1Kv8Mn=LSH*g2R<;!B{S&kLh~}Iy%EX!>SM`gtc^=X5fUY0a4-ByFjF} z@Wk*Va#qv>A{++@jF6^XjtHG-7=AVo5voTKK=@7iY&*r&3*nw}-^+zB?owo+MR*LI zV~s4IiG#b>5KID<$RJKTk7O|QV!(yp?(zu!&||u9Cq;(6B$(sjdlWGnVe^sa&>kpQ z1XDyR;xGjOL5-buj10^wfR;0|S;YfoKxYCBw+zpn&OjT{l^D@O&UCs!Kc%@M*9hza zN1&&AV-#hLwM!A&4Wov#IZPo8ARYv05VgxOq^V*I!KdAu0SJ>p>J-S8f-aAwg(Lzg zf`cLkTI&vcq)G$GgkKI4gFGCyAyd@Sgd|;*zod`f!R_QxD!l-Ef?2O%9 zV=sqtCuG3RBUK0S5S~xw$la!OZmt8YT`3q5kWe#ZJAeqQbOCTub_YOjVT-b+)CEEp z1E#^q7Ob5?M=(L9934ESuv0RK(XopM`2_tkhTNfM2(AGJ8$z-qnluFfBb@`xDQb0k zjhtn{uhA(`#Ba zx&v@xcT!LXum~v3!P*&wU(TQ~~Akn5jZM zu-d{tG6e*PQN*8I%Q0iv9OB=876}aT)iVpLE010ks{JKdW~+wvY1~lmLq4Q zSRbuM^{@!?T9mum&Jd_qT;Gj)R~d==nF-`XG{5MT+d(PU0ZpK_0%-!Rr9~4sTQP`- zzTBT<0m1z>orDT)hm4gU@_^C3=Mlzkb|Gsv9 zjqN7hWGh7N=b@nODs&8NEAtSJ#6)eYrh$a5i*c_SYga7 z4`p=JiUSeCS|q~*6I0kM{17tPzF+GgnMQI}g#ZyP?z^766UpffrqCk4=+T5DqW^=L zyIRj#2@bF_C$sV{O*jqC5|Nk}B0lwy@Izn~H05lhhooC2)J>B}ovx+A8}(?SvIwt< zoJm$cyGTX}ctd2La<14zV)h9(;b2|zL@|&lKLjPcW{)Od2+)+N$ixH^t#gj&(h%gR z0b*JhV1duBbwIKfgb*kLOSg3vIatz#{9QCBc@8Mc$SY|g%|bwz@vnzO^rP4j{#~uZ zhN#JSv)d*o^B5yeTtkQCfka-*{4x^>W{*$U5%N~#Ju(6BBAM!7G?8|=Gz60bri93k zWD%}M6V`#LYltBg^(cv?Ye2QoVMoY0F!UUKm&JieT1NoMJEWv}gcHM}gJX<1mx7K` zNG_TX4_lv?V0Zh@iX!OIGdQG#3@&^aou_lK+Y!7x3rUcY@8A>jBu)e{gO#fkJwCxD zXFte`9a2Jh57|9x0io~k$sh@D13a=*dyiOq#W*1z$t1SRC;c&I_!uL(p4US{jj>X~ zMKg!iabAV4Gc|WaLo};*9)Meil(3KpS(2>Wd)wtRgZX06k#f7FglZcnVR~L*mnSPV z+;f3iqa$hq$yyO@)1jYOI%AAQzg#{UNK6e_ZjdGIZr{Ns#yp@;c8t+%&W&1`Lzi-l zCT4+@m6fK9fN!~fn#bzQ2l^ObE{Vd^oF&p8l#T=4)!wi_G6KrdB^^R2I?{!*R>MCQ zf!)xM3~(q#co9@XWFIIb1ab#J4+^z}cZU$>RBn6Fw`yLs1cX=UK|vLhe+<){O@sof z3ve(^x!f`bI@u8*_XYQ$$iG;whYy#Tm4u=;3)`GiUK3Cb0V}!AA%J7lwQJ_pc6DK< zMt9FUgvaSIfYJ;MRM*)>Sm;7Q4fX`<#JfXXklrwFVfG-`ReDt6n&_Kxhs?!#P*^8) z1#uSLvrB+wG^}qz59dUY9#u>z&^6W%GvUa4P&f&y0E}B!w0ltI1Neh*~`PgAG}39RQoC1~Vl?z059GZPehCEIWpPu?+qc0R=*EP-QlZ z){#ocNZ#!h;$DCl!O@cGRtxOr9^P(bRS zYqXfdVF<9{V@}#3sItn=+L%kV8ByUc=;LMh>rzGIm}F>samf|oLg(bLFIj@^QRTuo zdQY`+3@oT3nbHZrtQ7UAf>A_GI?dccl_@-}1%oRSl`d5Xl#>$VAK21sS3)Gmx+Ds? z%N1j{Dd(>k2{>;nRrm?uw0+Mhzjt_r=7AhF5Z$snB0_X1pfj`w&d`UTzyXaxu1ED4 zK-7k4X)_Kbq+0QhzDQP1dQ?%Fg~}puVwz+Z7G%YVhfbCq-GaHoymW+-OVeEju#n8m zpOzr2CjyF66bu_q)2=Qdr#)~RRBySY)1%5nK-Z_SGtD6ZR?NTv{1L(ie;L{(#q<(- z5}?*?=KZJjk4PmHUB?Q@oFzF4rR(7kIl4^-p#Jngj`T82-ZH62K&9laxGn>5W>kL` zc^qaMLP1#Np#OEL6D7%s0`ee7#-!r}*Nd>|pE@iWd3d1r@S)t|&}aHy{2>Q`_Spv> zJx6LWskhjZD_symuOJA_eyvnVk%P*(Cb$1|hJbTUOn{i=!)4g1qG4|G=T-$rddX=J zscMzAOF-{EK)`~^26B^rmseUd_~rVULkU?xqct;V%E@<^DzhaDjA~%_zl`Y2E+=Jf ztFX8z#dxJ1t6c*0Jap6KSC&sZywVD}6#)`J9JVq*hswPgEKNElujmpYI`c(Pc2K3h z(yieZhjFu#gOWm^ONn;xf&zP}_a*2^Xzdoa`D(RhxVF!6baoF0xY8>Z1Uho117dE& zT@-TNxC3Q+%jqz42Durf3&nu4_sl}vP%a5$jthOYDJPc8-k zMY_O6S63RuVjf*ArsZ>%#5)q8o>Ha=nxU*EccIfEKs*X$`LTs!8S=2faOtI5{O;2i zYk{k8Y1bEnCeGJmZ&Uee~wN>CpMq4p{#nm?j13TSqd~q#WPuGjB z%{2?Q{b?^&tns_eMyzccbZ`7@uqgbrmZ8qTR#>|9_$$dKE&axqvjP)aM{!z**?NW_ zBA2z?+-G1hVC!oeb+DNa*GF4fCBm(NZGRu@Rij}T4~4I7v6B@_s25FwH4<6P=wdn9 zL}D;8Ofl>2Vc~A+?J%2|Q>yD>(Kh!(Ge*e$LlVnSPM)P%NGxVOIy%VxuRSc=hgH%w zC%(+=#p0iN%$wH99Vjwhh$)t z=}(U?c-8J4JjiX|T`cE*q^)C+LuRaNv0B2ELC$M?bfgmMJz9(K%^Y7pn^!m<*!-j}PMHDAG*9O3J-U;i5^yps*AoR+j~jFn z6H>dLF<(I4fLWfqZ?H3F15BIbDs7Jk;-EJi4_OzKSO!j=n2PMZht-iCG%z*DEXV ztDM!zwcie1R@DN=D;)>yab8#8xF=I5>vkSEq2p5NBwj4eJ?SU4t#f^WdTGgxG?pc6 zJ*R-ocJ>oEo>0qpCY(}_wBK%9TKug=`gia#Z#b6uejoncb^icE$%Egn{`zkZ z)QWm$a7n3f3VmHL@YLy5oL+iZJ*Mylp9c7}=HcbjpC&e*V$WoF_SuR$y?k|0-k@+M zgZ}XR;2DGNne0#01bY|x<8i3&=Beos;oW59Om<>Iro-Nu&;0%be-llvJUD@VLtVg! z`_6w^f7RXp^2&Rc)i?O$Hu5s}(ttP3zw~`xnSaT07w_plGS1v+Iop`n*$lct_E14I zfqFA&v*4HJCh%3a8FX+5B=E=!>ic)TO{v%XE53dIPW4)KXYdPe3;*MjZ~ekQ{KgBP z{OVskWIR)^-84|1<7Uv4ZwG;s?cwQ9@4)NZC+Aik!~1(qo_qtpuJHZ(K#lKEE7yKz z75aVc4v6zX{d)VgpI5<&@pHfT(T{v<|6?Ej?vDBdpNBYTpXO(@JR6z+OIt=62z9=G zbK53k>Mjl$5(dlaegF6p;=$4^THHB{7VRa31l7t@r;-TmKdsv)wH)Dtig)Ty16Anv z*~{|u!<@^w2~hyo@t@J~_<^D;|BxDlyys!Q;QMc$14&uTxO(vN>^^@T@Hs=u3XysR^U+XhSOYY6nGW?xUg zV~nq>2+tqB-t!DU+O|n7c`)e}iC)*+pu%~a+a?EpOgSi|6EEQvTJb-W58m5jDlg;G z64JLD#{^sTgFRg(tH|73GjA%?273)eCW{P}3o1e!UEr<$uJSH84SgJ5{?AF!yzpCN zrgDQ5=bsq7b7xLeRUg!V6{XJWJDKb8Xep?cg1SLOeQOVx+X?heQMO90;xB;jwGbwXMOtAQ{$zj@!-XkH}Wcul?lK2 zWIzLYU|tmc;&oi$AE&vabPe2O}GV7DD`HvN|wHWzf;}^Zb;=Ur1C?szp|vp zgQXW+hJbq;{!I3j52ko-;FKRlqJq=cf7q2DIk-RyMo-DWOn&OcRw8X12DC2}Qkonu zU*Nj|JzLDB`oegO31}BO(~H}u{W3*B=*RiOV-#Qxji?y4Yvah+~g!k zswqj(nsdXA_GVCW1fK3j&P%_1x zj4ztx!!s_?3=ZA>@b3Z^#_(9gpw5f&0+p+(*Gqd+(IRuZ`~=i*xhXW)HtUxSMvR zWjqn7Dcp{%IH3TG?bztPPHu*DktvZ&O@rSiJ)Cg4`l)-^0q}6Mho-=nH?^NSA8v+D z=Z0JV(rxlV&R_f~)nw^xgW&v{zgL5|Bhy^@-k)B*ss<;o{Vg#1_~i%me1~b^!)kDi z-#+)bc@KkMW-CAPL+N!JOQ*UV;@kAU5Lg!%rY9!M4PKsnoxUvs_s(B_cB8Bh`bEIk zzizsSi1o(pUGdy;^D-8oYc3uQ98nLC z#}~%PcGY8#KB|6*M2E+hbb0F?gTaL_E}c;C!5L2XKA^7+Vs&cr;bpu%ioYkO$ItS> zoDToqi#txvn)-JF)gwwR>2{R*;@|{MnXbj3-&qfGE`o_1FW! zsR85_tIsLEfBt)qJ^m@g5PH)2i1Lp8UwhBfkM2LuVgA8ntk0i?lT=|ddj32Tu9GL% z)VtN-`N6C3PAA}M<=cPx&!2thUDxgy41Vpu|9F7? HGLQcS*hbRi literal 0 HcmV?d00001