From mboxrd@z Thu Jan  1 00:00:00 1970
Subject: Re: [edk2-devel] [PATCH 1/3 v7] Drivers/OpTeeRpmb: Add an OP-TEE backed RPMB driver
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>,devel@edk2.groups.io
From: "Sami Mujawar" <sami.mujawar@arm.com>
X-Originating-Location: GB (217.140.106.53)
X-Originating-Platform: Windows Firefox 86
User-Agent: GROUPS.IO Web Poster
MIME-Version: 1.0
Date: Sat, 27 Mar 2021 13:25:41 -0700
References: <20210313183139.3256104-2-ilias.apalodimas@linaro.org>
In-Reply-To: <20210313183139.3256104-2-ilias.apalodimas@linaro.org>
Message-ID: <14174.1616876741028136783@groups.io>
Content-Type: multipart/alternative; boundary="y2moOgUNdMAgobbDd02F"

--y2moOgUNdMAgobbDd02F
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi Ilias,

On Sat, Mar 13, 2021 at 10:31 AM, Ilias Apalodimas wrote:

>=20
> + NumBytes =3D3D NumLba * Instance->BlockSize;=3D0D
> + Base =3D3D (VOID *)Instance->MemBaseAddress + Start * Instance->BlockS=
iz=3D
> e;=3D0D
> + Buf =3D3D AllocatePool (NumLba * Instance->BlockSize);=3D0D
> + if (Buf =3D3D=3D3D NULL) {=3D0D
> + return EFI_DEVICE_ERROR;=3D0D
> + }=3D0D
> + SetMem64 (Buf, NumLba * Instance->BlockSize, ~0UL);=3D0D
> + // Write the device=3D0D
> + Status =3D3D ReadWriteRpmb (=3D0D
> + SP_SVC_RPMB_WRITE,=3D0D
> + (UINTN)Buf,=3D0D
> + NumBytes,=3D0D
> + Start * Instance->BlockSize=3D0D
> + );=3D0D
> + if (EFI_ERROR (Status)) {=3D0D

I think there is a memory leak here as Buf is not freed. If you agree, I w=
ill fix this locally before merging the changes.

>=20
> + return Status;=3D0D
> + }=3D0D
> + // Update the in memory copy=3D0D
> + SetMem64 (Base, NumLba * Instance->BlockSize, ~0UL);=3D0D
> + FreePool (Buf);=3D0D
> + }=3D0D
> +=3D0D
> + VA_END (Args);=3D0D
> +=3D0D
> + return EFI_SUCCESS;=3D0D
> +}=3D0D

Regards,

Sami Mujawar

--y2moOgUNdMAgobbDd02F
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi Ilias,<br /><br />On Sat, Mar 13, 2021 at 10:31 AM, Ilias Apalodimas wro=
te:<br />
<blockquote>+ NumBytes =3D3D NumLba * Instance-&gt;BlockSize;=3D0D<br />+ =
Base =3D3D (VOID *)Instance-&gt;MemBaseAddress + Start * Instance-&gt;Block=
Siz=3D<br />e;=3D0D<br />+ Buf =3D3D AllocatePool (NumLba * Instance-&gt;Bl=
ockSize);=3D0D<br />+ if (Buf =3D3D=3D3D NULL) {=3D0D<br />+ return EFI_DEV=
ICE_ERROR;=3D0D<br />+ }=3D0D<br />+ SetMem64 (Buf, NumLba * Instance-&gt;B=
lockSize, ~0UL);=3D0D<br />+ // Write the device=3D0D<br />+ Status =3D3D R=
eadWriteRpmb (=3D0D<br />+ SP_SVC_RPMB_WRITE,=3D0D<br />+ (UINTN)Buf,=3D0D<=
br />+ NumBytes,=3D0D<br />+ Start * Instance-&gt;BlockSize=3D0D<br />+ );=
=3D0D<br />+ if (EFI_ERROR (Status)) {=3D0D</blockquote>
I think there is a memory leak here as Buf is not freed. If you agree, I w=
ill fix this locally before merging the changes.<br />
<blockquote>+ return Status;=3D0D<br />+ }=3D0D<br />+ // Update the in me=
mory copy=3D0D<br />+ SetMem64 (Base, NumLba * Instance-&gt;BlockSize, ~0UL=
);=3D0D<br />+ FreePool (Buf);=3D0D<br />+ }=3D0D<br />+=3D0D<br />+ VA_END=
 (Args);=3D0D<br />+=3D0D<br />+ return EFI_SUCCESS;=3D0D<br />+}=3D0D</blo=
ckquote>
Regards,<br /><br />Sami Mujawar

--y2moOgUNdMAgobbDd02F--