From marka@isc.org Tue Jan 24 21:34:22 2017 In-Reply-To: Your message of "Tue, 24 Jan 2017 16:20:17 -0000." X-Spam-Status: No, score=-4.3 required=5.0 tests=ALL_TRUSTED,BAYES_00, RP_MATCHES_RCVD autolearn=ham autolearn_force=no version=3.4.0 X-RT-Interface: API References: <1485274809.7324.3.camel@redhat.com> content-type: text/plain; charset="utf-8" Message-ID: <20170124213414.D4498608270B@rock.dv.isc.org> X-RT-Original-Encoding: utf-8 Received: from mx.pao1.isc.org (mx.pao1.isc.org [149.20.64.53]) by bugs.isc.org (Postfix) with ESMTP id C7BC571B5A8 for ; Tue, 24 Jan 2017 21:34:21 +0000 (UTC) Received: from zmx1.isc.org (zmx1.isc.org [149.20.0.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx.pao1.isc.org (Postfix) with ESMTPS id AA5F9349422 for ; Tue, 24 Jan 2017 21:34:19 +0000 (UTC) Received: from zmx1.isc.org (localhost [127.0.0.1]) by zmx1.isc.org (Postfix) with ESMTPS id 9A8B816006F for ; Tue, 24 Jan 2017 21:34:19 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by zmx1.isc.org (Postfix) with ESMTP id 8CE2016006D for ; Tue, 24 Jan 2017 21:34:19 +0000 (UTC) Received: from zmx1.isc.org ([127.0.0.1]) by localhost (zmx1.isc.org [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id gImiI7mCDib8 for ; Tue, 24 Jan 2017 21:34:19 +0000 (UTC) Received: from rock.dv.isc.org (c27-253-115-14.carlnfd2.nsw.optusnet.com.au [27.253.115.14]) by zmx1.isc.org (Postfix) with ESMTPSA id C2E5D160067 for ; Tue, 24 Jan 2017 21:34:18 +0000 (UTC) Received: from rock.dv.isc.org (localhost [IPv6:::1]) by rock.dv.isc.org (Postfix) with ESMTP id D4498608270B for ; Wed, 25 Jan 2017 08:34:14 +1100 (EST) Delivered-To: bind9-bugs@bugs.isc.org Subject: Re: [ISC-Bugs #44514] Dead code introduced by -P5 and -P2 patches Return-Path: X-Original-To: bind9-bugs@bugs.isc.org Date: Wed, 25 Jan 2017 08:34:14 +1100 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on mx.pao1.isc.org To: bind9-bugs@isc.org From: "Mark Andrews" RT-Message-ID: Content-Length: 3672 We are aware of the issue. This is the proposed fix. This will be part of the next Coverity run (roughly weekly). Mark diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 397048e..d02a5e7 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -6834,10 +6834,10 @@ answer_response(fetchctx_t *fctx) { dns_name_t *name, *dname = NULL, *qname, tname, *ns_name; dns_name_t *cname = NULL; dns_rdataset_t *rdataset, *ns_rdataset; - isc_boolean_t done, external, chaining, aa, found, want_chaining; + isc_boolean_t done, external, aa, found, want_chaining; isc_boolean_t have_answer, found_cname, found_dname, found_type; isc_boolean_t wanted_chaining; - unsigned int aflag; + unsigned int aflag, chaining; dns_rdatatype_t type; dns_fixedname_t fdname, fqname; dns_view_t *view; @@ -6855,9 +6855,9 @@ answer_response(fetchctx_t *fctx) { found_cname = ISC_FALSE; found_dname = ISC_FALSE; found_type = ISC_FALSE; - chaining = ISC_FALSE; have_answer = ISC_FALSE; want_chaining = ISC_FALSE; + chaining = 0; POST(want_chaining); if ((message->flags & DNS_MESSAGEFLAG_AA) != 0) aa = ISC_TRUE; @@ -7004,7 +7004,7 @@ answer_response(fetchctx_t *fctx) { rdataset->attributes |= DNS_RDATASETATTR_CACHE; rdataset->trust = dns_trust_answer; - if (!chaining) { + if (chaining == 0) { /* * This data is "the" answer * to our question only if @@ -7081,8 +7081,8 @@ answer_response(fetchctx_t *fctx) { * cause us to ignore the signatures of * CNAMEs. */ - if (wanted_chaining) - chaining = ISC_TRUE; + if (wanted_chaining && chaining < 2U) + chaining++; } else { dns_rdataset_t *dnameset = NULL; @@ -7113,7 +7113,7 @@ answer_response(fetchctx_t *fctx) { * If we're not chaining, then the DNAME and * its signature should not be external. */ - if (!chaining && external) { + if (chaining == 0 && external) { char qbuf[DNS_NAME_FORMATSIZE]; char obuf[DNS_NAME_FORMATSIZE]; @@ -7197,7 +7197,14 @@ answer_response(fetchctx_t *fctx) { name->attributes |= DNS_NAMEATTR_CACHE; rdataset->attributes |= DNS_RDATASETATTR_CACHE; rdataset->trust = dns_trust_answer; - if (!chaining) { + /* + * If we are not chaining or the first CNAME + * is a synthesised CNAME before the DNAME. + */ + if ((chaining == 0) || + (chaining == 1U && + namereln == dns_namereln_commonancestor)) + { /* * This data is "the" answer to * our question only if we're @@ -7245,8 +7252,12 @@ answer_response(fetchctx_t *fctx) { dnameset->attributes |= DNS_RDATASETATTR_CHAINING; } - if (wanted_chaining) - chaining = ISC_TRUE; + /* + * Ensure that we can't ever get chaining == 1 + * above if we have processed a DNAME. + */ + if (wanted_chaining && chaining < 2U) + chaining += 2; } result = dns_message_nextname(message, DNS_SECTION_ANSWER); } @@ -7271,7 +7282,7 @@ answer_response(fetchctx_t *fctx) { /* * Did chaining end before we got the final answer? */ - if (chaining) { + if (chaining != 0) { /* * Yes. This may be a negative reply, so hand off * authority section processing to the noanswer code. @@ -7320,7 +7331,7 @@ answer_response(fetchctx_t *fctx) { DNS_NAMEATTR_CACHE; rdataset->attributes |= DNS_RDATASETATTR_CACHE; - if (aa && !chaining) + if (aa && chaining == 0) rdataset->trust = dns_trust_authauthority; else -- Mark Andrews, ISC 1 Seymour St., Dundas Valley, NSW 2117, Australia PHONE: +61 2 9871 4742 INTERNET: marka@isc.org