From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by mx.groups.io with SMTP id smtpd.web11.25140.1684499373051553953 for ; Fri, 19 May 2023 05:29:33 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=U/WimwbO; spf=pass (domain: gmail.com, ip: 209.85.214.173, mailfrom: pedro.falcato@gmail.com) Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1ae452c2777so5972875ad.0 for ; Fri, 19 May 2023 05:29:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684499372; x=1687091372; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/vfBDb1n5QTZuzJAONQ9VDbxUOfHafLPfeSQEPRdooQ=; b=U/WimwbO4QymCMmjS9k7yni06S5YvpqAAzUkttdRT5o0oHDu91ynNZ3aVopkVII5QV l8Z47neRNfVd4xNEFiqcQ8/BSPBMiP6KrKtODUudRVfVYnNbjSlP48vEn4kEmAnXPL5p dj5MQ8jmdodpZnQECyHWeVHkAoMjOzVO1uPWuI+ggSfB1+VuXE88wQawP+gBlHS2DvVG hwStc7X/zaV0YXxjreYeMjRQm3kwgsTXvLUY0opJJFO8+zlYkF8l5auR8aM9+qGbSb+R pxAgAAVL+ITzL1AQ6BXuRHg+mn2zxymElFOO/kgNgQb2SjnQ/idHrpg/ovjBTCA/WMth MYTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684499372; x=1687091372; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/vfBDb1n5QTZuzJAONQ9VDbxUOfHafLPfeSQEPRdooQ=; b=A/Voz78MxWEApE93OzQpl+a3nH5IlpnJTnJCCopDZZAK2XqEdr8/YyJXV3ViRVI51a dVinp1A3TnxkgYD2wAac4Vmgs2m8Bvju8CLNf7RJprQeY3rSQao25NoQQNhzIP/synSv RrkEjwkGMz3ril5adOuvm21l9l+RWmPdbs20aHqJwlFXILHka5zcwPOVKBwQ9uE9C4K/ BdkQRPrK8iVF9EqOrq+IOegqmYS8cYdcV0acHHT1BtRm9+RTjh1ty0CPL2R9HTYQX64k YhdPSgphe7LjI5G7/HFHFuNXf9g68xAWPVuLk8/xTl32dISAXlGUTI8z1t9VIPYF/8RO vMYw== X-Gm-Message-State: AC+VfDyuz4sTCQTNEDeXaU8mobazDkj1SnqpNPNRBoH4tPQ5MVtoqz2j toWNim7+TDjsQ/chTnDB457dlUUSloHvlIpaYhCgp7+8ois= X-Google-Smtp-Source: ACHHUZ6NjB5kOoSl7Gt5URJ4FL1O+yhQSgpn73w3arGaJJoyQ0Q84EV6twoHRZEDnBgeXQMltFaqhH4Tqlq8c2HaKxc= X-Received: by 2002:a17:902:ecc8:b0:1ab:1260:19de with SMTP id a8-20020a170902ecc800b001ab126019demr2958535plh.11.1684499372194; Fri, 19 May 2023 05:29:32 -0700 (PDT) MIME-Version: 1.0 References: <20230518062851.184724-1-rsingh@ventanamicro.com> In-Reply-To: <20230518062851.184724-1-rsingh@ventanamicro.com> From: "Pedro Falcato" Date: Fri, 19 May 2023 13:29:21 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 1/1] IntelFsp2Pkg/Library/BaseFspCommonLib: Fix OVERRUN Coverity issue To: devel@edk2.groups.io, rsingh@ventanamicro.com Cc: Chasel Chiu , Nate DeSimone , Star Zeng , Ranbir Singh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, May 18, 2023 at 4:16=E2=80=AFPM Ranbir Singh wrote: > > FspData->PerfIdx is getting increased for every call unconditionally > in the function SetFspMeasurePoint and hence memory access can happen > for out of bound FspData->PerfData[] array entries also. > > Example - > FspData->PerfData is an array of 32 UINT64 entries. Assume a call > is made to SetFspMeasurePoint function when the FspData->PerfIdx > last value is 31. It gets incremented to 32 at line 400. > Any subsequent call to SetFspMeasurePoint functions leads to > FspData->PerfData[32] getting accessed which is out of the PerfData > array as well as the FSP_GLOBAL_DATA structure boundary. > > Hence keep array access and index increment inside if block only and > return invalid performance timestamp when PerfIdx is invalid. > > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Star Zeng > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4200 > Signed-off-by: Ranbir Singh > Signed-off-by: Ranbir Singh > --- > IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c b/Intel= Fsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > index a22b0e7825ad..cda2a7b2478e 100644 > --- a/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > +++ b/IntelFsp2Pkg/Library/BaseFspCommonLib/FspCommonLib.c > @@ -377,7 +377,8 @@ GetFspSiliconInitUpdDataPointer ( > > @param[in] Id Measurement point ID. > > - @return performance timestamp. > + @return performance timestamp if current PerfIdx is valid, > + else return 0 as invalid performance timestamp > **/ > UINT64 > EFIAPI > @@ -395,9 +396,10 @@ SetFspMeasurePoint ( > if (FspData->PerfIdx < sizeof (FspData->PerfData) / sizeof (FspData->P= erfData[0])) { > FspData->PerfData[FspData->PerfIdx] =3D AsmReadTsc = (); > ((UINT8 *)(&FspData->PerfData[FspData->PerfIdx]))[7] =3D Id; > + return FspData->PerfData[(FspData->PerfIdx)++]; > } > > - return FspData->PerfData[(FspData->PerfIdx)++]; > + return (UINT64)0x0000000000000000; return 0; Works just as well. You also don't need a cast. https://godbolt.org/z/e5vvGcWWo --=20 Pedro