From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web11.7090.1616164033824659203 for ; Fri, 19 Mar 2021 07:27:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=VD0PT5PJ; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 9826C64F4D for ; Fri, 19 Mar 2021 14:27:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1616164032; bh=iY8AKu/L9QUk/uW4rwELzPQMkk8t6c1+Dy2vqhju6mc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=VD0PT5PJ+r5QTZzOH+tcCZzWwpW8yObMWuwVt9w7DfWIzxHCLPOvOzYzUEVhIUFkX C0rsguqHj0WaXwgF8CYIVi8OT8TlqhEXmHO07QWAUUxFhjntuJYBHj3+0Sk/lAU7ui Id/iQ3mbd3MiiNbu2wv4Ig+ba7yVNq9F8KMsk4IcpRiaPwpfNWkSjNWCRaiEb7yJ/o MsrtVGhA0dDUQ1/OA+xBRy73RNiqGitoA9H51bLa+BaY6w/8ta0AFEW2cigeCfDHUh 46ZHB6KZhoGbKqR8IUxGQHX31bFZsQm+x53ipBFU7aMLjnJaGZsXmY7lQd29xdg3Xk FcEKK5r+bwxPA== Received: by mail-oi1-f169.google.com with SMTP id m13so4942477oiw.13 for ; Fri, 19 Mar 2021 07:27:12 -0700 (PDT) X-Gm-Message-State: AOAM531Xa4HOQn6uRsXYz+EHyzU6Fw2vrCvaODDI6hPaKkhkxMQzbzLc ylTnVtsINh5h5yoSbE1Wu51G4393QGcUBPiQLvQ= X-Google-Smtp-Source: ABdhPJwOGXMs7vKzHWhik1JKFxf5WiOmLMtUlSQ9qD12jjftzDWAK9AMiSqA2zKv7EQ3yo4UI+QKSbsDYONNylhRTis= X-Received: by 2002:aca:ab86:: with SMTP id u128mr1190132oie.47.1616164031814; Fri, 19 Mar 2021 07:27:11 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: "Ard Biesheuvel" Date: Fri, 19 Mar 2021 15:27:00 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v1] OvmfPkg/X86QemuLoadImageLib: Handle allocation failure for CommandLine To: Martin Radev Cc: devel@edk2.groups.io, Laszlo Ersek , Ard Biesheuvel , Jordan Justen , Tom Lendacky Content-Type: text/plain; charset="UTF-8" On Thu, 18 Mar 2021 at 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 > > Signed-off-by: Martin Radev This seems reasonable to me. Acked-by: Ard Biesheuvel > --- > 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)); > + > + 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 ...")); > -- > 2.17.1 >