From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-x233.google.com (mail-oi0-x233.google.com [IPv6:2607:f8b0:4003:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8479B1A1E46 for ; Tue, 25 Oct 2016 04:09:54 -0700 (PDT) Received: by mail-oi0-x233.google.com with SMTP id i127so37671446oia.2 for ; Tue, 25 Oct 2016 04:09:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=+A6R36fpR7nAQ1EjjlGyRkegw6/Tg54++K+1eJp3y2s=; b=MPGt0Aa+FpmwHET5chqBf4bYMPDk+3nzvwRRyuHLeFcpeATN3MTQscrnaQjpFRcOew aobUFVxQp+a6msth8OSf7l2gpe9M7i5tMNrBNxiNNEPTtw2R1kHKJ33GZTNw+sca0F/D xVYhCMXPo7B6C6RZcXgKxhE699nRNHBv2Uetc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=+A6R36fpR7nAQ1EjjlGyRkegw6/Tg54++K+1eJp3y2s=; b=h0QlDfVqlnl0WZQ5o5XKaGJhvvqX55PtedTZ341J3RvppdW3hy6TMv4m8jpSjdkCek sL+GIMrpLLmxPMhhR1Z83L43OJ7XNMJ5ZslXr8HFb3AndimsJpO0S1x4dFWshlhdwAmy CTLDoG4/0shoUR0GMur6NeBnP1401pExAxE1dgHM3GhboQj06mQAYlwAJef9Xo5InbHC /rBJr3k8BfVbBEuf/o09QF7K2IsktcQRaVwe088gNKTvifKj+S3r8T4PZTzVoS3d52p7 DhvAI8ffPcocXR9RVm1sv6rvY6dfLsD5u7sXvvx5Myt/5a75IZAwiGZqbA/4ughs10pl o3jg== X-Gm-Message-State: ABUngvdQVJLHnNcLovIvs2nLLZRxxi11+Apk+JAgcfduHl4yPhWfpxd283xyDu3LSu2Qz+D7saop2HXCrmmnC6pp X-Received: by 10.107.2.65 with SMTP id 62mr15045508ioc.83.1477393793762; Tue, 25 Oct 2016 04:09:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.5.139 with HTTP; Tue, 25 Oct 2016 04:09:53 -0700 (PDT) In-Reply-To: References: <1477325206-24646-1-git-send-email-ard.biesheuvel@linaro.org> <1477325206-24646-4-git-send-email-ard.biesheuvel@linaro.org> <4819b517-89e8-3c61-ebb7-a718ae4be909@redhat.com> From: Ard Biesheuvel Date: Tue, 25 Oct 2016 12:09:53 +0100 Message-ID: To: Ryan Harkin Cc: Laszlo Ersek , "edk2-devel@lists.01.org" , Leif Lindholm 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 11:09:54 -0000 Content-Type: text/plain; charset=UTF-8 On 25 October 2016 at 12:08, Ryan Harkin wrote: > On 25 October 2016 at 11:56, Ard Biesheuvel wrote: >> On 25 October 2016 at 11:53, Laszlo Ersek wrote: >>> 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"). >>> >> >> The LinuxLoader is an unmaintained piece of junk, and will be removed >> as soon as we can. > > Who still uses it? Hikey? > I certainly hope not. HiKey is AARCH64, and LinuxLoader is quite badly broken on that architecture. > >> I did notice that none of these ATAG functions >> check whether the allocation as a whole is not overrun, but fixing >> /that/ goes way beyond what we're willing to do in terms of >> maintenance on deprecated code. >> >>>> 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, >> Ard. >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel