public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io, michael.d.kinney@intel.com,
	Erich McMillan <emcmillan@microsoft.com>
Cc: "Dong, Eric" <eric.dong@intel.com>,
	"Kumar, Rahul R" <rahul.r.kumar@intel.com>,
	"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally uninitialized variables
Date: Fri, 24 Mar 2023 11:50:39 -0400	[thread overview]
Message-ID: <e5fd4fbb-4246-4f8c-70b4-7f0b6b4ec83d@linux.microsoft.com> (raw)
In-Reply-To: <d6130a8f-8a5c-3cce-13fb-0ce757678182@linux.microsoft.com>

Hi Mike,

Would you like a change here?

Thanks,
Michael

On 3/21/2023 10:25 AM, Michael Kubacki wrote:
> I spoke to Erich offline and he mentioned that a previous coding 
> practice he used specified unsigned integer literals when intended so he 
> applied that here.
> 
> Thanks,
> Michael
> 
> On 3/10/2023 5:59 PM, Michael Kubacki wrote:
>> Erich introduced this particular change, he may be able to provide 
>> more info.
>>
>> On 3/10/2023 3:03 PM, Michael D Kinney wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
>>>> Michael Kubacki
>>>> Sent: Friday, March 10, 2023 10:43 AM
>>>> To: devel@edk2.groups.io
>>>> Cc: Dong, Eric <eric.dong@intel.com>; Erich McMillan 
>>>> <emcmillan@microsoft.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Michael Kubacki 
>>>> <mikuback@linux.microsoft.com>; Kumar, Rahul R
>>>> <rahul.r.kumar@intel.com>; Ni, Ray <ray.ni@intel.com>
>>>> Subject: [edk2-devel] [PATCH v4 10/12] UefiCpuPkg: Fix conditionally 
>>>> uninitialized variables
>>>>
>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> Fixes CodeQL alerts for CWE-457:
>>>> https://cwe.mitre.org/data/definitions/457.html
>>>>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Erich McMillan <emcmillan@microsoft.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Co-authored-by: Erich McMillan <emcmillan@microsoft.com>
>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>> ---
>>>>   UefiCpuPkg/CpuMpPei/CpuBist.c   | 8 +++++++-
>>>>   UefiCpuPkg/CpuMpPei/CpuMpPei.c  | 8 +++++++-
>>>>   UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 ++++++++-
>>>>   3 files changed, 22 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuBist.c 
>>>> b/UefiCpuPkg/CpuMpPei/CpuBist.c
>>>> index 7dc93cd784d4..122808139b87 100644
>>>> --- a/UefiCpuPkg/CpuMpPei/CpuBist.c
>>>> +++ b/UefiCpuPkg/CpuMpPei/CpuBist.c
>>>> @@ -175,7 +175,13 @@ CollectBistDataFromPpi (
>>>>     EFI_SEC_PLATFORM_INFORMATION_RECORD2  *PlatformInformationRecord2;
>>>>     EFI_SEC_PLATFORM_INFORMATION_CPU      *CpuInstanceInHob;
>>>>
>>>> -  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, 
>>>> &NumberOfEnabledProcessors);
>>>> +  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, 
>>>> &NumberOfEnabledProcessors);
>>>> +  ASSERT_EFI_ERROR (Status);
>>>> +
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    NumberOfProcessors        = 1u;
>>>> +    NumberOfEnabledProcessors = 1u;
>>>
>>> Why is '1u' used instead of '1'?  I don't see a lot of usage of the 
>>> postfix
>>> unless the const needs to be forced to a larger bitwidth than default 
>>> int type.
>>>
>>>> +  }
>>>>
>>>>     BistInformationSize = sizeof 
>>>> (EFI_SEC_PLATFORM_INFORMATION_RECORD2) +
>>>>                           sizeof (EFI_SEC_PLATFORM_INFORMATION_CPU) 
>>>> * NumberOfProcessors;
>>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c 
>>>> b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>>>> index e7f1fe9f426c..a84304273168 100644
>>>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>>>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>>>> @@ -473,7 +473,13 @@ InitializeMpExceptionStackSwitchHandlers (
>>>>       return;
>>>>     }
>>>>
>>>> -  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
>>>> +  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
>>>> +  ASSERT_EFI_ERROR (Status);
>>>> +
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    NumberOfProcessors = 1u;
>>>> +  }
>>>> +
>>>>     SwitchStackData = AllocatePages (EFI_SIZE_TO_PAGES 
>>>> (NumberOfProcessors * sizeof
>>>> (EXCEPTION_STACK_SWITCH_CONTEXT)));
>>>>     ASSERT (SwitchStackData != NULL);
>>>>     ZeroMem (SwitchStackData, NumberOfProcessors * sizeof 
>>>> (EXCEPTION_STACK_SWITCH_CONTEXT));
>>>> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c 
>>>> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>>>> index 135422225340..1322fcb77f28 100644
>>>> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
>>>> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>>>> @@ -538,6 +538,7 @@ SetupStackGuardPage (
>>>>     UINTN                 NumberOfProcessors;
>>>>     UINTN                 Bsp;
>>>>     UINTN                 Index;
>>>> +  EFI_STATUS            Status;
>>>>
>>>>     //
>>>>     // One extra page at the bottom of the stack is needed for Guard 
>>>> page.
>>>> @@ -547,7 +548,13 @@ SetupStackGuardPage (
>>>>       ASSERT (FALSE);
>>>>     }
>>>>
>>>> -  MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
>>>> +  Status = MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
>>>> +  ASSERT_EFI_ERROR (Status);
>>>> +
>>>> +  if (EFI_ERROR (Status)) {
>>>> +    NumberOfProcessors = 1u;
>>>> +  }
>>>> +
>>>>     MpInitLibWhoAmI (&Bsp);
>>>>     for (Index = 0; Index < NumberOfProcessors; ++Index) {
>>>>       StackBase = 0;
>>>> -- 
>>>> 2.39.2.windows.1
>>>>
>>>>
>>>>
>>>> -=-=-=-=-=-=
>>>> Groups.io Links: You receive all messages sent to this group.
>>>> View/Reply Online (#101030): 
>>>> https://edk2.groups.io/g/devel/message/101030
>>>> Mute This Topic: https://groups.io/mt/97526805/1643496
>>>> Group Owner: devel+owner@edk2.groups.io
>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub 
>>>> [michael.d.kinney@intel.com]
>>>> -=-=-=-=-=-=
>>>>
>>>
>>>
>>>
>>> 
>>>
>>>

  reply	other threads:[~2023-03-24 15:50 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 18:42 [PATCH v4 00/12] Enable New CodeQL Queries Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 02/12] BaseTools/PatchCheck.py: Add PCCTS to tab exemption list Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 03/12] BaseTools/VfrCompile: Fix potential buffer overwrites Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 04/12] CryptoPkg: Fix conditionally uninitialized variable Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 05/12] MdeModulePkg: Fix conditionally uninitialized variables Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 06/12] MdePkg: " Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 07/12] NetworkPkg: " Michael Kubacki
2023-03-10 20:01   ` Michael D Kinney
2023-03-10 18:42 ` [PATCH v4 08/12] PcAtChipsetPkg: " Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 09/12] ShellPkg: " Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 10/12] UefiCpuPkg: " Michael Kubacki
2023-03-10 20:03   ` [edk2-devel] " Michael D Kinney
2023-03-10 22:59     ` Michael Kubacki
2023-03-21 14:25       ` Michael Kubacki
2023-03-24 15:50         ` Michael Kubacki [this message]
2023-03-24 15:54           ` Michael D Kinney
2023-03-10 18:42 ` [PATCH v4 11/12] .github/codeql/edk2.qls: Enable CWE 457, 676, and 758 queries Michael Kubacki
2023-03-10 18:42 ` [PATCH v4 12/12] .github/codeql/edk2.qls: Enable CWE 120, 787, and 805 queries Michael Kubacki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e5fd4fbb-4246-4f8c-70b4-7f0b6b4ec83d@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox