On Fri, Aug 16, 2019 at 4:35 PM Dumitru Ceara <[email protected]> wrote: > > On Fri, Aug 16, 2019 at 4:32 PM Dumitru Ceara <[email protected]> wrote: > > > > From: Dumitru Ceara <[email protected]> > > > > Due to the use of a uint8_t to index inside the DNS payload we could end > > up in an infinite loop when specific (invalid) DNS packets were > > processed by ovn-controller. In the infinite loop we keep increasing the > > query_name dynamic string until running out of memory. > > > > One way to replicate the issue is to configure DNS on the logical switch > > and then inject a manually crafted DNS-like packet. For example, with > > Scapy: > > > > >>> p = IP(dst='10.0.0.2',src='10.0.0.3')/UDP(dport=53)/('a'*364) > > >>> send(p) > > > > Also add a sanity check on minimum L4 size of packets. > > > > CC: Numan Siddique <[email protected]> > > Fixes: 16cb4fb8ca49 ("ovn-controller: Add 'dns_lookup' action") > > Reported-at: https://bugzilla.redhat.com/1740335 > > Reported-by: Priscila <[email protected]> > > Signed-off-by: Dumitru Ceara <[email protected]> > > Signed-off-by: Numan Siddique <[email protected]> > > > > (cherry-picked from ovn commit - 7fbdeaade826da299c20c05050627ebea65fe8c2) > > --- > > ovn/controller/pinctrl.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > > index e443449..e388bbc 100644 > > --- a/ovn/controller/pinctrl.c > > +++ b/ovn/controller/pinctrl.c > > @@ -1628,6 +1628,12 @@ pinctrl_handle_dns_lookup( > > goto exit; > > } > > > > + /* Check that the packet stores at least the minimal headers. */ > > + if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN + DNS_HEADER_LEN)) { > > + VLOG_WARN_RL(&rl, "truncated dns packet"); > > + goto exit; > > + } > > + > > /* Extract the DNS header */ > > struct dns_header const *in_dns_header = > > dp_packet_get_udp_payload(pkt_in); > > if (!in_dns_header) { > > @@ -1652,7 +1658,7 @@ pinctrl_handle_dns_lookup( > > uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len); > > uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1); > > uint8_t *in_queryname = in_dns_data; > > - uint8_t idx = 0; > > + uint16_t idx = 0; > > struct ds query_name; > > ds_init(&query_name); > > /* Extract the query_name. If the query name is - 'www.ovn.org' it > > would be > > -- > > 1.8.3.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > Hi, > > Please backport this to 2.11, 2.10, 2.9 at least. > > Thanks, > Dumitru
Hi Ben, It would be great if we could have this bug fix in the 2.12 branch before the release. The fix is already in OVN master. Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
