From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 B63321A1E46 for ; Tue, 25 Oct 2016 03:53:15 -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 2EE217AE99; Tue, 25 Oct 2016 10:53:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-71.phx2.redhat.com [10.3.116.71]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9PArDjc030576; Tue, 25 Oct 2016 06:53:14 -0400 To: Ard Biesheuvel , edk2-devel@ml01.01.org, leif.lindholm@linaro.org References: <1477325206-24646-1-git-send-email-ard.biesheuvel@linaro.org> <1477325206-24646-4-git-send-email-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: <4819b517-89e8-3c61-ebb7-a718ae4be909@redhat.com> Date: Tue, 25 Oct 2016 12:53:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1477325206-24646-4-git-send-email-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 25 Oct 2016 10:53:15 +0000 (UTC) Subject: Re: [PATCH 3/6] ArmPkg/LinuxLoader: eliminate calls to deprecated string functions 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, 25 Oct 2016 10:53:15 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 10/24/16 18:06, Ard Biesheuvel wrote: > Remove calls to deprecated string functions like AsciiStrCpy() and > UnicodeStrToAsciiStr() > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel > --- > ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c | 2 +- > ArmPkg/Application/LinuxLoader/LinuxLoader.c | 6 ++++-- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c > index fd7ee9c8624d..0b3e2489c758 100644 > --- a/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c > +++ b/ArmPkg/Application/LinuxLoader/Arm/LinuxAtag.c > @@ -72,7 +72,7 @@ SetupCmdlineTag ( > mLinuxKernelCurrentAtag->header.type = ATAG_CMDLINE; > > /* place CommandLine into tag */ > - AsciiStrCpy (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, CmdLine); > + AsciiStrCpyS (mLinuxKernelCurrentAtag->body.cmdline_tag.cmdline, LineLength, CmdLine); > > // move pointer to next tag > mLinuxKernelCurrentAtag = next_tag_address (mLinuxKernelCurrentAtag); Apparently nothing in this file checks if the tags being added still actually fit in the preallocated space (which is EFI_SIZE_TO_PAGES (ATAG_MAX_SIZE)). The change does preserve the previous behavior ("copy the full string"). > diff --git a/ArmPkg/Application/LinuxLoader/LinuxLoader.c b/ArmPkg/Application/LinuxLoader/LinuxLoader.c > index 70b960b66f0e..76697c3a8c9d 100644 > --- a/ArmPkg/Application/LinuxLoader/LinuxLoader.c > +++ b/ArmPkg/Application/LinuxLoader/LinuxLoader.c > @@ -61,6 +61,7 @@ LinuxLoaderEntryPoint ( > LIST_ENTRY *ResourceLink; > SYSTEM_MEMORY_RESOURCE *Resource; > EFI_PHYSICAL_ADDRESS SystemMemoryBase; > + UINTN Length; > > Status = gBS->LocateProtocol ( > &gEfiDevicePathFromTextProtocolGuid, > @@ -182,12 +183,13 @@ LinuxLoaderEntryPoint ( > } > > if (LinuxCommandLine != NULL) { > - AsciiLinuxCommandLine = AllocatePool ((StrLen (LinuxCommandLine) + 1) * sizeof (CHAR8)); > + Length = StrLen (LinuxCommandLine) + 1; > + AsciiLinuxCommandLine = AllocatePool (Length); > if (AsciiLinuxCommandLine == NULL) { > Status = EFI_OUT_OF_RESOURCES; > goto Error; > } > - UnicodeStrToAsciiStr (LinuxCommandLine, AsciiLinuxCommandLine); > + UnicodeStrToAsciiStrS (LinuxCommandLine, AsciiLinuxCommandLine, Length); > } > > // > I prefer to call character counts without the terminating NUL "Length", and character counts with the terminating NUL "Size", but that's just a personal preference. (And the rest of this code uses Length differently already, for example in "LineLength" itself, near the top.) Reviewed-by: Laszlo Ersek Thanks Laszlo