From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.1634.1616179752577973702 for ; Fri, 19 Mar 2021 11:49:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=LG5DbopS; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1616179751; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ak0sGkoYeH/I/0XAHUq7ILWh866BheLD2zrdfhhNdHE=; b=LG5DbopSsCsKn01I+uY1ZQPUJImRS1XZbpXmqvsT45Grf9sSFp5BrEaF6u6BGb+gOHVs1C 1gSaxohns0Van1v2BQC2aw7oUdaPtzneB6ym5VnEBcLTz84fNQGrwy33OrW5xY4cnEZpvD LTJhfA0yQfvP8ksfli5h6iyHaJ8hJ5k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-91-pqlixvYNN-i0xbcMraGlMQ-1; Fri, 19 Mar 2021 14:49:06 -0400 X-MC-Unique: pqlixvYNN-i0xbcMraGlMQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6CC6718C8C00; Fri, 19 Mar 2021 18:49:05 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-31.ams2.redhat.com [10.36.114.31]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1CE0D5D6D5; Fri, 19 Mar 2021 18:49:03 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v1] OvmfPkg/X86QemuLoadImageLib: Handle allocation failure for CommandLine To: devel@edk2.groups.io, martin.b.radev@gmail.com Cc: ardb+tianocore@kernel.org, jordan.l.justen@intel.com, thomas.lendacky@amd.com References: From: "Laszlo Ersek" Message-ID: <837d416d-c5ce-77ab-6a25-3648807aa468@redhat.com> Date: Fri, 19 Mar 2021 19:49:03 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 03/18/21 22:44, Martin Radev wrote: > The CommandLine and InitrdData may be set to NULL if the provided > size is too large. Because the zero page is mapped, this would not > cause an immediate crash but can lead to memory corruption instead. > This patch just adds validation and returns error if either allocation > has failed. > > Ref: https://github.com/martinradev/edk2/commit/6c0ce748b97393240c006e24b73652f30e597a05 > (1) I've dropped this reference from the commit message, as the referred-to object appears ephemeral. > Signed-off-by: Martin Radev > --- > OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c > index 931553c0c1..b983c4d7d0 100644 > --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c > +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c > @@ -161,6 +161,12 @@ QemuLoadLegacyImage ( > LoadedImage->CommandLine = LoadLinuxAllocateCommandLinePages ( > EFI_SIZE_TO_PAGES ( > LoadedImage->CommandLineSize)); > + (2) I've dropped this empty line too. Reviewed-by: Laszlo Ersek Merged as commit ca3188827140, via , with Ard's and Tom's ACKs. Thanks for the fix! Laszlo > + if (LoadedImage->CommandLine == NULL) { > + DEBUG ((DEBUG_ERROR, "Unable to allocate memory for kernel command line!\n")); > + Status = EFI_OUT_OF_RESOURCES; > + goto FreeImage; > + } > QemuFwCfgSelectItem (QemuFwCfgItemCommandLineData); > QemuFwCfgReadBytes (LoadedImage->CommandLineSize, LoadedImage->CommandLine); > } > @@ -178,6 +184,11 @@ QemuLoadLegacyImage ( > LoadedImage->InitrdData = LoadLinuxAllocateInitrdPages ( > LoadedImage->SetupBuf, > EFI_SIZE_TO_PAGES (LoadedImage->InitrdSize)); > + if (LoadedImage->InitrdData == NULL) { > + DEBUG ((DEBUG_ERROR, "Unable to allocate memory for initrd!\n")); > + Status = EFI_OUT_OF_RESOURCES; > + goto FreeImage; > + } > DEBUG ((DEBUG_INFO, "Initrd size: 0x%x\n", > (UINT32)LoadedImage->InitrdSize)); > DEBUG ((DEBUG_INFO, "Reading initrd image ...")); >