From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C3F662117D295 for ; Tue, 23 Oct 2018 01:52:37 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Oct 2018 01:52:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,415,1534834800"; d="scan'208";a="83699084" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.11]) ([10.239.9.11]) by orsmga008.jf.intel.com with ESMTP; 23 Oct 2018 01:52:35 -0700 To: Hao Wu , edk2-devel@lists.01.org Cc: Liangcheng Tang , Star Zeng References: <20181018064200.2068-1-hao.a.wu@intel.com> <20181018064200.2068-2-hao.a.wu@intel.com> From: "Ni, Ruiyu" Message-ID: <1b418f0c-968a-ca84-5fc2-2b00a7ddf94c@Intel.com> Date: Tue, 23 Oct 2018 16:53:51 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181018064200.2068-2-hao.a.wu@intel.com> Subject: Re: [PATCH v1 1/3] MdeModulePkg/NvmExpressDxe: Refine data buffer & len check in PassThru X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Oct 2018 08:52:37 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/18/2018 2:41 PM, Hao Wu wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1142 > > According to the the NVM Express spec Revision 1.1, for some commands > (like Get/Set Feature Command, Figure 89 & 90 of the spec), the Memory > Buffer maybe optional although the command opcode indicates there is a > data transfer between host & controller (Get/Set Feature Command, Figure > 38 of the spec). > > Hence, this commit refine the checks for the 'TransferLength' and > 'TransferBuffer' field of the EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET > structure to address this issue. > > Cc: Liangcheng Tang > Cc: Ruiyu Ni > Cc: Star Zeng > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Hao Wu > --- > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 33 +++++++++++--------- > 1 file changed, 18 insertions(+), 15 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c > index 2468871322..bfcd349794 100644 > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c > @@ -595,7 +595,8 @@ NvmExpressPassThru ( > // > if (((Sq->Opc & (BIT0 | BIT1)) != 0) && > !((Packet->QueueType == NVME_ADMIN_QUEUE) && ((Sq->Opc == NVME_ADMIN_CRIOCQ_CMD) || (Sq->Opc == NVME_ADMIN_CRIOSQ_CMD)))) { > - if ((Packet->TransferLength == 0) || (Packet->TransferBuffer == NULL)) { > + if (((Packet->TransferLength != 0) && (Packet->TransferBuffer == NULL)) || > + ((Packet->TransferLength == 0) && (Packet->TransferBuffer != NULL))) { > return EFI_INVALID_PARAMETER; > } > > @@ -605,21 +606,23 @@ NvmExpressPassThru ( > Flag = EfiPciIoOperationBusMasterWrite; > } > > - MapLength = Packet->TransferLength; > - Status = PciIo->Map ( > - PciIo, > - Flag, > - Packet->TransferBuffer, > - &MapLength, > - &PhyAddr, > - &MapData > - ); > - if (EFI_ERROR (Status) || (Packet->TransferLength != MapLength)) { > - return EFI_OUT_OF_RESOURCES; > - } > + if ((Packet->TransferLength != 0) && (Packet->TransferBuffer != NULL)) { > + MapLength = Packet->TransferLength; > + Status = PciIo->Map ( > + PciIo, > + Flag, > + Packet->TransferBuffer, > + &MapLength, > + &PhyAddr, > + &MapData > + ); > + if (EFI_ERROR (Status) || (Packet->TransferLength != MapLength)) { > + return EFI_OUT_OF_RESOURCES; > + } > > - Sq->Prp[0] = PhyAddr; > - Sq->Prp[1] = 0; > + Sq->Prp[0] = PhyAddr; > + Sq->Prp[1] = 0; > + } > > if((Packet->MetadataLength != 0) && (Packet->MetadataBuffer != NULL)) { > MapLength = Packet->MetadataLength; > Reviewed-by: Ruiyu Ni -- Thanks, Ray