From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf0-x229.google.com (mail-lf0-x229.google.com [IPv6:2a00:1450:4010:c07::229]) (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 8C3781A1E46 for ; Tue, 25 Oct 2016 04:08:21 -0700 (PDT) Received: by mail-lf0-x229.google.com with SMTP id b81so201399034lfe.1 for ; Tue, 25 Oct 2016 04:08:21 -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=p+ZKu9zv3edpPFbX/x+j4uR1ReuLYJwT/6zlrhntY4Q=; b=fDKamGpN+5waAt1SeE+N3rRNXnlNaEh4wO0dq0x7gi7LtIyhqAhP3VQaiMrQqzBlsn gmCHAsnbLQbmwKMbMNVy5/kTPPUqNX3SEftB2M+szc4zUphiN3WHLE/FOzl7tnTE019V IEz8mbw+1qxjFQ6z7Vs3M/IsQTN+4bjRRt4F4= 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=p+ZKu9zv3edpPFbX/x+j4uR1ReuLYJwT/6zlrhntY4Q=; b=IZsY1w3Jokd9R+v3EbkSLlmt6oHVENMihny69eDN73qOQQccdx+RSjfIbeTRAFe5d0 iunO5FxxusR57hz7bvz0yuNmo1631eYuMdZ8eQ+08CMXOM5AjiJZ0cKNGKo48Pna9J+Q gm79R3qrEVD0k9prTSyqhw6lyPedUnscNmxtTdm/ZjSdiHguY2RAB1sVZauO2OGFkfSX WG4ml/Fmik7k6CycEaqqrZOZoDyskfjOnxdC8Pa6ktOVvDAIyFpU7c2Q5SyfeAoXXqIk dqhqpjS+HDSJm8WojviAPjH+gXY+dTBcTmllLaeGayKbPAjJCUUrqLKso+aLKDgV0hLx 1NpQ== X-Gm-Message-State: ABUngvfER7jowCrt3Mx4ZFEmLRK+lIAMT7BNpFUYz77PbsfNSM8uMwAFzKPMzzrIHC2GF4gzaM8HsqNnb8F5TFBN X-Received: by 10.25.7.201 with SMTP id 192mr10261560lfh.170.1477393699271; Tue, 25 Oct 2016 04:08:19 -0700 (PDT) MIME-Version: 1.0 Received: by 10.25.28.78 with HTTP; Tue, 25 Oct 2016 04:08:18 -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: Ryan Harkin Date: Tue, 25 Oct 2016 12:08:18 +0100 Message-ID: To: Ard Biesheuvel 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:08:22 -0000 Content-Type: text/plain; charset=UTF-8 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 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