From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6F96E1A1DEF for ; Mon, 5 Sep 2016 20:55:05 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 05 Sep 2016 20:55:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,290,1470726000"; d="scan'208";a="1046077228" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga002.jf.intel.com with ESMTP; 05 Sep 2016 20:55:05 -0700 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 5 Sep 2016 20:55:04 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx121.amr.corp.intel.com (10.18.125.36) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 5 Sep 2016 20:55:04 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.109]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.250]) with mapi id 14.03.0248.002; Tue, 6 Sep 2016 11:55:01 +0800 From: "Fu, Siyuan" To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" CC: Hegde Nagaraj P , "Ye, Ting" Thread-Topic: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server Thread-Index: AQHSB/A680gOXJGtQUiYsT6xYolsDaBr09ZA Date: Tue, 6 Sep 2016 03:55:01 +0000 Message-ID: References: <1473133142-41256-1-git-send-email-jiaxin.wu@intel.com> <1473133142-41256-2-git-send-email-jiaxin.wu@intel.com> In-Reply-To: <1473133142-41256-2-git-send-email-jiaxin.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjQ0OTNhZmMtZjM1ZC00MTIyLWEwMTctYzhmNGRhMGI4YzRiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlE5U1JyV2preW5SakRHUnJrSFlIaGFKQWR2NUpNbERpTElaNDNiKzlrcFE9In0= x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the name server X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Sep 2016 03:55:05 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, Jiaxin I have 2 comments for the patch. 1. I think we can add the CNAME record to the DNS cache, just like the A an= d AAAA records, so the driver won't need to repeat the query if user query = the host again. 2. We should hold the original query packet until the response is parsed su= ccessfully, otherwise the system will crash again if a retransmit is needed= . Best Regards Siyuan > -----Original Message----- > From: Wu, Jiaxin > Sent: Tuesday, September 6, 2016 11:39 AM > To: edk2-devel@lists.01.org > Cc: Hegde Nagaraj P ; Fu, Siyuan > ; Ye, Ting > Subject: [Patch] NetworkPkg/DnsDxe: Handle CNAME type responded from the > name server >=20 > According RFC 1034 - 3.6.2, if the query name is an alias, the name serve= r > will include the CNAME record in the response and restart the query at th= e > domain name specified in the data field of the CNAME record. RFC also > provides > one example server action when A query received: >=20 > Suppose a name server was processing a query with for USCISIC.ARPA, askin= g > for > type A information, and had the following resource records: > USC-ISIC.ARPA IN CNAME C.ISI.EDU > C.ISI.EDU IN A 10.0.0.52 > Both of these RRs would be returned in the response to the type A query. >=20 > Currently, DnsDxe driver doesn't handle the CNAME type response, which > will cause > any exception result. The driver need continue the packet parsing while > CNAME type > record parsed. So, this patch is used to handle it correctly. >=20 > Cc: Hegde Nagaraj P > Cc: Fu Siyuan > Cc: Ye Ting > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiaxin Wu > --- > NetworkPkg/DnsDxe/DnsImpl.c | 60 ++++++++++++++++++++++++++-------------= - > ----- > 1 file changed, 35 insertions(+), 25 deletions(-) >=20 > diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c > index 360f68e..c68ec88 100644 > --- a/NetworkPkg/DnsDxe/DnsImpl.c > +++ b/NetworkPkg/DnsDxe/DnsImpl.c > @@ -1231,17 +1231,10 @@ ParseDnsResponse ( > if (DnsHeader->Flags.Bits.RCode !=3D DNS_FLAGS_RCODE_NO_ERROR || > DnsHeader->AnswersNum < 1 || \ > DnsHeader->Flags.Bits.QR !=3D DNS_FLAGS_QR_RESPONSE) { > Status =3D EFI_ABORTED; > goto ON_EXIT; > } > - > - // > - // Free the sending packet. > - // > - if (Item->Value !=3D NULL) { > - NetbufFree ((NET_BUF *) (Item->Value)); > - } >=20 > // > // Do some buffer allocations. > // > if (Instance->Service->IpVersion =3D=3D IP_VERSION_4) { > @@ -1288,40 +1281,42 @@ ParseDnsResponse ( > // > // It's the GeneralLookUp querying. > // > Dns6TokenEntry->Token->RspData.GLookupData =3D AllocatePool (sizeo= f > (DNS_RESOURCE_RECORD)); > if (Dns6TokenEntry->Token->RspData.GLookupData =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > Dns6TokenEntry->Token->RspData.GLookupData->RRList =3D AllocatePoo= l > (DnsHeader->AnswersNum * sizeof (DNS_RESOURCE_RECORD)); > if (Dns6TokenEntry->Token->RspData.GLookupData->RRList =3D=3D NULL= ) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > } else { > // > // It's not the GeneralLookUp querying. Check the Query type. > // > if (QuerySection->Type =3D=3D DNS_TYPE_AAAA) { > Dns6TokenEntry->Token->RspData.H2AData =3D AllocatePool (sizeof > (DNS6_HOST_TO_ADDR_DATA)); > if (Dns6TokenEntry->Token->RspData.H2AData =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > Dns6TokenEntry->Token->RspData.H2AData->IpList =3D AllocatePool > (DnsHeader->AnswersNum * sizeof (EFI_IPv6_ADDRESS)); > if (Dns6TokenEntry->Token->RspData.H2AData->IpList =3D=3D NULL) = { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > } else { > Status =3D EFI_UNSUPPORTED; > goto ON_EXIT; > } > } > } >=20 > + Status =3D EFI_NOT_FOUND; > + > // > // Processing AnswerSection. > // > while (AnswerSectionNum < DnsHeader->AnswersNum) { > // > @@ -1348,51 +1343,53 @@ ParseDnsResponse ( > // > // Fill the ResourceRecord. > // > Dns4RR[RRCount].QName =3D AllocateZeroPool (AsciiStrLen (QueryName= ) + > 1); > if (Dns4RR[RRCount].QName =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns4RR[RRCount].QName, QueryName, AsciiStrLen (QueryName)= ); > Dns4RR[RRCount].QType =3D AnswerSection->Type; > Dns4RR[RRCount].QClass =3D AnswerSection->Class; > Dns4RR[RRCount].TTL =3D AnswerSection->Ttl; > Dns4RR[RRCount].DataLength =3D AnswerSection->DataLength; > Dns4RR[RRCount].RData =3D AllocateZeroPool > (Dns4RR[RRCount].DataLength); > if (Dns4RR[RRCount].RData =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns4RR[RRCount].RData, AnswerData, > Dns4RR[RRCount].DataLength); >=20 > RRCount ++; > + Status =3D EFI_SUCCESS; > } else if (Instance->Service->IpVersion =3D=3D IP_VERSION_6 && > Dns6TokenEntry->GeneralLookUp) { > Dns6RR =3D Dns6TokenEntry->Token->RspData.GLookupData->RRList; > AnswerData =3D (UINT8 *) AnswerSection + sizeof (*AnswerSection); >=20 > // > // Fill the ResourceRecord. > // > Dns6RR[RRCount].QName =3D AllocateZeroPool (AsciiStrLen (QueryName= ) + > 1); > if (Dns6RR[RRCount].QName =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns6RR[RRCount].QName, QueryName, AsciiStrLen (QueryName)= ); > Dns6RR[RRCount].QType =3D AnswerSection->Type; > Dns6RR[RRCount].QClass =3D AnswerSection->Class; > Dns6RR[RRCount].TTL =3D AnswerSection->Ttl; > Dns6RR[RRCount].DataLength =3D AnswerSection->DataLength; > Dns6RR[RRCount].RData =3D AllocateZeroPool > (Dns6RR[RRCount].DataLength); > if (Dns6RR[RRCount].RData =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns6RR[RRCount].RData, AnswerData, > Dns6RR[RRCount].DataLength); >=20 > RRCount ++; > + Status =3D EFI_SUCCESS; > } else { > // > // It's not the GeneralLookUp querying. > // Check the Query type, parse the response packet. > // > @@ -1425,30 +1422,31 @@ ParseDnsResponse ( > // > // Allocate new CacheEntry pool. > // > Dns4CacheEntry =3D AllocateZeroPool (sizeof (EFI_DNS4_CACHE_ENTR= Y)); > if (Dns4CacheEntry =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > Dns4CacheEntry->HostName =3D AllocateZeroPool (2 * > (StrLen(Dns4TokenEntry->QueryHostName) + 1)); > if (Dns4CacheEntry->HostName =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns4CacheEntry->HostName, Dns4TokenEntry->QueryHostName= , > 2 * (StrLen(Dns4TokenEntry->QueryHostName) + 1)); > Dns4CacheEntry->IpAddress =3D AllocateZeroPool (sizeof > (EFI_IPv4_ADDRESS)); > if (Dns4CacheEntry->IpAddress =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns4CacheEntry->IpAddress, AnswerData, sizeof > (EFI_IPv4_ADDRESS)); > Dns4CacheEntry->Timeout =3D AnswerSection->Ttl; >=20 > UpdateDns4Cache (&mDriverData->Dns4CacheList, FALSE, TRUE, > *Dns4CacheEntry); >=20 > - IpCount ++; > + IpCount ++; > + Status =3D EFI_SUCCESS; > break; > case DNS_TYPE_AAAA: > // > // This is address entry, get Data. > // > @@ -1476,30 +1474,38 @@ ParseDnsResponse ( > // > // Allocate new CacheEntry pool. > // > Dns6CacheEntry =3D AllocateZeroPool (sizeof (EFI_DNS6_CACHE_ENTR= Y)); > if (Dns6CacheEntry =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > Dns6CacheEntry->HostName =3D AllocateZeroPool (2 * > (StrLen(Dns6TokenEntry->QueryHostName) + 1)); > if (Dns6CacheEntry->HostName =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns6CacheEntry->HostName, Dns6TokenEntry->QueryHostName= , > 2 * (StrLen(Dns6TokenEntry->QueryHostName) + 1)); > Dns6CacheEntry->IpAddress =3D AllocateZeroPool (sizeof > (EFI_IPv6_ADDRESS)); > if (Dns6CacheEntry->IpAddress =3D=3D NULL) { > - Status =3D EFI_UNSUPPORTED; > + Status =3D EFI_OUT_OF_RESOURCES; > goto ON_EXIT; > } > CopyMem (Dns6CacheEntry->IpAddress, AnswerData, sizeof > (EFI_IPv6_ADDRESS)); > Dns6CacheEntry->Timeout =3D AnswerSection->Ttl; >=20 > UpdateDns6Cache (&mDriverData->Dns6CacheList, FALSE, TRUE, > *Dns6CacheEntry); >=20 > IpCount ++; > + Status =3D EFI_SUCCESS; > + break; > + case DNS_TYPE_CNAME: > + // > + // According RFC 1034 - 3.6.2, if the query name is an alias, th= e > name server will include the CNAME > + // record in the response and restart the query at the domain > name specified in the data field of the > + // CNAME record. So, let's skip this CNAME record parsing and > continue the next one. > + // > break; > default: > Status =3D EFI_UNSUPPORTED; > goto ON_EXIT; > } > @@ -1539,24 +1545,28 @@ ParseDnsResponse ( > } > } > } >=20 > // > - // Parsing is complete, SignalEvent here. > + // Parsing is complete, free the sending packet and signal Event here. > // > + if (Item !=3D NULL && Item->Value !=3D NULL) { > + NetbufFree ((NET_BUF *) (Item->Value)); > + } > + > if (Instance->Service->IpVersion =3D=3D IP_VERSION_4) { > ASSERT (Dns4TokenEntry !=3D NULL); > Dns4RemoveTokenEntry (&Instance->Dns4TxTokens, Dns4TokenEntry); > - Dns4TokenEntry->Token->Status =3D EFI_SUCCESS; > + Dns4TokenEntry->Token->Status =3D Status; > if (Dns4TokenEntry->Token->Event !=3D NULL) { > gBS->SignalEvent (Dns4TokenEntry->Token->Event); > DispatchDpc (); > } > } else { > ASSERT (Dns6TokenEntry !=3D NULL); > Dns6RemoveTokenEntry (&Instance->Dns6TxTokens, Dns6TokenEntry); > - Dns6TokenEntry->Token->Status =3D EFI_SUCCESS; > + Dns6TokenEntry->Token->Status =3D Status; > if (Dns6TokenEntry->Token->Event !=3D NULL) { > gBS->SignalEvent (Dns6TokenEntry->Token->Event); > DispatchDpc (); > } > } > -- > 1.9.5.msysgit.1