From patchwork Tue Apr 26 12:42:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 66695 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp1587327qge; Tue, 26 Apr 2016 05:42:27 -0700 (PDT) X-Received: by 10.98.95.4 with SMTP id t4mr3290098pfb.100.1461674547336; Tue, 26 Apr 2016 05:42:27 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id d10si3318864pag.20.2016.04.26.05.42.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 26 Apr 2016 05:42:27 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id DF85C1A1E6B; Tue, 26 Apr 2016 05:42:26 -0700 (PDT) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 0EB031A1E60 for ; Tue, 26 Apr 2016 05:42:25 -0700 (PDT) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9A3B580099; Tue, 26 Apr 2016 12:42:24 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-90.phx2.redhat.com [10.3.113.90]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3QCgFNb004617; Tue, 26 Apr 2016 08:42:23 -0400 From: Laszlo Ersek To: edk2-devel-01 Date: Tue, 26 Apr 2016 14:42:12 +0200 Message-Id: <1461674532-28993-3-git-send-email-lersek@redhat.com> In-Reply-To: <1461674532-28993-1-git-send-email-lersek@redhat.com> References: <1461674532-28993-1-git-send-email-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 Subject: [edk2] [PATCH 2/2] OvmfPkg: SataControllerDxe: SataControllerStop: fix use after free X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Ruiyu Ni , Jordan Justen , wang xiaofeng MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" It would be possible to remove the UAF without local variables, by calling SataPrivateData->PciIo->Attributes() before releasing SataPrivateData. However, by keeping the location of the call (for which temporary variables are necessary), we continue to match the error path logic in SataControllerStart(), which is always recommended. Reported-by: wang xiaofeng Fixes: bcab71413407e61c144994925556725dd65eede9 Cc: wang xiaofeng Cc: Jordan Justen Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- OvmfPkg/SataControllerDxe/SataController.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) -- 1.8.3.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/OvmfPkg/SataControllerDxe/SataController.c b/OvmfPkg/SataControllerDxe/SataController.c index e5ee63a0ab63..1f84ad034e5b 100644 --- a/OvmfPkg/SataControllerDxe/SataController.c +++ b/OvmfPkg/SataControllerDxe/SataController.c @@ -563,90 +563,95 @@ EFIAPI SataControllerStop ( IN EFI_DRIVER_BINDING_PROTOCOL *This, IN EFI_HANDLE Controller, IN UINTN NumberOfChildren, IN EFI_HANDLE *ChildHandleBuffer ) { EFI_STATUS Status; EFI_IDE_CONTROLLER_INIT_PROTOCOL *IdeInit; EFI_SATA_CONTROLLER_PRIVATE_DATA *SataPrivateData; + EFI_PCI_IO_PROTOCOL *PciIo; + UINT64 OriginalPciAttributes; // // Open the produced protocol // Status = gBS->OpenProtocol ( Controller, &gEfiIdeControllerInitProtocolGuid, (VOID **) &IdeInit, This->DriverBindingHandle, Controller, EFI_OPEN_PROTOCOL_GET_PROTOCOL ); if (EFI_ERROR (Status)) { return EFI_UNSUPPORTED; } SataPrivateData = SATA_CONTROLLER_PRIVATE_DATA_FROM_THIS (IdeInit); ASSERT (SataPrivateData != NULL); + PciIo = SataPrivateData->PciIo; + OriginalPciAttributes = SataPrivateData->OriginalPciAttributes; + // // Uninstall the IDE Controller Init Protocol from this instance // Status = gBS->UninstallMultipleProtocolInterfaces ( Controller, &gEfiIdeControllerInitProtocolGuid, &(SataPrivateData->IdeInit), NULL ); if (EFI_ERROR (Status)) { return Status; } if (SataPrivateData->DisqualifiedModes != NULL) { FreePool (SataPrivateData->DisqualifiedModes); } if (SataPrivateData->IdentifyData != NULL) { FreePool (SataPrivateData->IdentifyData); } if (SataPrivateData->IdentifyValid != NULL) { FreePool (SataPrivateData->IdentifyValid); } FreePool (SataPrivateData); // // Restore original PCI attributes // - SataPrivateData->PciIo->Attributes ( - SataPrivateData->PciIo, - EfiPciIoAttributeOperationSet, - SataPrivateData->OriginalPciAttributes, - NULL - ); + PciIo->Attributes ( + PciIo, + EfiPciIoAttributeOperationSet, + OriginalPciAttributes, + NULL + ); // // Close protocols opened by Sata Controller driver // return gBS->CloseProtocol ( Controller, &gEfiPciIoProtocolGuid, This->DriverBindingHandle, Controller ); } /** Calculate the flat array subscript of a (Channel, Device) pair. @param[in] SataPrivateData The private data structure corresponding to the SATA controller that attaches the device for which the flat array subscript is being calculated. @param[in] Channel The channel (ie. port) number on the SATA controller that the device is attached to. @param[in] Device The device number on the channel. @return The flat array subscript suitable for indexing DisqualifiedModes, IdentifyData, and IdentifyValid. **/