From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id B518B74003E for ; Thu, 23 May 2024 15:24:36 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=WzjnLtVXfgGxXiENB98r3ZOPjO+wj2QSFGikP4biRjc=; c=relaxed/simple; d=groups.io; h=Received-SPF:Authentication-Results-Original:Message-ID:Date:User-Agent:Subject:To:Cc:References:From:In-Reply-To:MIME-Version:NoDisclaimer:Original-Authentication-Results:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Language; s=20240206; t=1716477875; v=1; b=nmoQa80Vsrq7336LbCtixgwoIz4vNEUaIgyh+NkdO2LnPz5g+fGmueK7VrwUg1XbwZiOfMQu ai1vtoH6gVL+LQALhbAhEBu6BFArORQkoKFypHtwcADWpLw3LeeJ0nahYrjrMRdJM0W2gv/cOzs IF9MKOWefjaUX46LtyYKTgtZu9rzxbcajEIlSxNPr/1DMhZMzEYchCszfW3pL1wwpqvy2jA5syq ISGNn1nRgkca59bV+EmJST5lKbQ+KZKpALqAivgiIilWbxl7cu23MmJu5mKsXGoTCKcb8hy8mnV jbDlqmd8FD2NVG2xocHUvmvlAHAqlBc28NO2ydvfK2V0Q== X-Received: by 127.0.0.2 with SMTP id 6KJGYY7687511xg3b4iGmCRq; Thu, 23 May 2024 08:24:35 -0700 X-Received: from EUR04-DB3-obe.outbound.protection.outlook.com (EUR04-DB3-obe.outbound.protection.outlook.com [40.107.6.73]) by mx.groups.io with SMTP id smtpd.web10.1437.1716477873509315591 for ; Thu, 23 May 2024 08:24:34 -0700 X-Received: from AM6P192CA0016.EURP192.PROD.OUTLOOK.COM (2603:10a6:209:83::29) by AM7PR08MB5528.eurprd08.prod.outlook.com (2603:10a6:20b:dd::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7611.22; Thu, 23 May 2024 15:24:29 +0000 X-Received: from AMS0EPF0000019B.eurprd05.prod.outlook.com (2603:10a6:209:83:cafe::80) by AM6P192CA0016.outlook.office365.com (2603:10a6:209:83::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7587.36 via Frontend Transport; Thu, 23 May 2024 15:24:29 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=arm.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C X-Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AMS0EPF0000019B.mail.protection.outlook.com (10.167.16.247) with Microsoft SMTP Server (version=TLS1_3, cipher=TLS_AES_256_GCM_SHA384) id 15.20.7611.14 via Frontend Transport; Thu, 23 May 2024 15:24:29 +0000 X-Received: ("Tessian outbound 2fd40f2ccfd7:v327"); Thu, 23 May 2024 15:24:28 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 0749b398290461e2 X-CR-MTA-TID: 64aa7808 X-Received: from 53bdb9bd4147.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id EFCEF6AA-701E-4D06-A939-521D2C3F1205.1; Thu, 23 May 2024 15:24:22 +0000 X-Received: from EUR04-DB3-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 53bdb9bd4147.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 23 May 2024 15:24:22 +0000 Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-Received: from PAXPR08MB6813.eurprd08.prod.outlook.com (2603:10a6:102:15f::10) by PAWPR08MB10092.eurprd08.prod.outlook.com (2603:10a6:102:369::5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7611.22; Thu, 23 May 2024 15:24:20 +0000 X-Received: from PAXPR08MB6813.eurprd08.prod.outlook.com ([fe80::610b:f3fd:2372:5c2]) by PAXPR08MB6813.eurprd08.prod.outlook.com ([fe80::610b:f3fd:2372:5c2%5]) with mapi id 15.20.7611.016; Thu, 23 May 2024 15:24:20 +0000 Message-ID: <5e2e64ef-dea6-4c6a-a6ee-9051a8599173@arm.com> Date: Thu, 23 May 2024 16:24:16 +0100 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [edk2-platforms][PATCH V3 11/17] Platform/ARM/NorFlashDxe: Fix memory leak in NorFlashCreateInstance() To: Sahil Kaushal , devel@edk2.groups.io Cc: Ard Biesheuvel , Leif Lindholm , sahil , "nd@arm.com" References: <20240523105511.13189-1-Sahil.Kaushal@arm.com> <20240523105511.13189-12-Sahil.Kaushal@arm.com> From: "Sami Mujawar" In-Reply-To: <20240523105511.13189-12-Sahil.Kaushal@arm.com> X-ClientProxiedBy: LO4P123CA0185.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1a4::10) To PAXPR08MB6813.eurprd08.prod.outlook.com (2603:10a6:102:15f::10) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: PAXPR08MB6813:EE_|PAWPR08MB10092:EE_|AMS0EPF0000019B:EE_|AM7PR08MB5528:EE_ X-MS-Office365-Filtering-Correlation-Id: 3cd9e9a4-f471-4e5e-738f-08dc7b3c703f x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0;ARA:13230031|376005|366007|1800799015; X-Microsoft-Antispam-Message-Info-Original: =?us-ascii?Q?SIrlSeBOXfOc82aUfDk+xuf1VpVQYAQD/u5UAUPdmVZFXmnhkmOE6VlHJ37Y?= =?us-ascii?Q?q2DVSpsubnCur9uFWmAB8ddMsy82y7QPILE6eq846hIHsHwoMRaoiwIz3ji8?= =?us-ascii?Q?xbWTvcLnBz1l9qN3rKLUUPXuaOXlCX9ASrQwpOtkCGz0DLv4FZ8qKOcxnV6h?= =?us-ascii?Q?SdF73JIfcUybPE5I/ky0Z+zJ0EUtjTmsUqed81i0q0rkIZxHx3zCiKrPnIdf?= =?us-ascii?Q?AJGMyOVlOysxJKJgOT9OoxyuZa8bGIntaWvnikYs3D9e8dJ0oFhfml3y2YCT?= =?us-ascii?Q?HExXDu8Ii3SyflnmCW9WPIoKe3QoPF52HMGSfC0HncIGSCkCNn2eGUVU33bS?= =?us-ascii?Q?4Q1Pl3G8vpP3QJpYSEFIjez/HilK6vdiVPRc5hdlnQMP8H/vvk7Qr54RVt7k?= =?us-ascii?Q?vgZvdsYW2zi6cR3mN98xMtYCTel+z3CZxM1cHaKxOOhK0kkp6HC2W9D+Q12d?= =?us-ascii?Q?daKb12ngrWzPyayI518nJuTx3BD6sbvA9H+MwkRv497IAbxT0PrUilIvFV4v?= =?us-ascii?Q?vQkLloQbRhAC6QnLrOYkg28WH4fyWkfXNepCDTonrJM6s13OLmevmX21TvB5?= =?us-ascii?Q?H57Dwh4VZemF0pVa8foDuUvDT8qwyL2YR2AS0Met3tI2GhRoLjX5SW3e2xrF?= =?us-ascii?Q?6VippKWIBd7ImVy+b9ub6q9Ju7qGiqV+OcI76u/UlTjo5yAiNszOuGbrx0Z6?= =?us-ascii?Q?MTxTE4ZJflPe01U2Rw94bJTDpC7mlCXgKw3nBa0PNyHrJRL8f8Kf9VsK0IPN?= =?us-ascii?Q?CyeIoN2pPu7ofvevd/orqCNerAkmxa+N0aNRN9WJWzAh3YZD8tRgqIXcBLYV?= =?us-ascii?Q?WZqaBuTZDn8SdA5D40lb9GK3Q+glLT3rI+QHyJJX2IVDTMv5ckngo9VeLO25?= =?us-ascii?Q?imIxA4UIxd5EYVm3fGoVqMmNPWeVTwQkNggSOCE4AnvWsQLiPZ3ZXYblLE3j?= =?us-ascii?Q?pF30w7ICcmYv+O2I506NlJUGsY7/hgPIYSSxd8AhFQoD7CYDGR7es7ueOhxY?= =?us-ascii?Q?dVhSP4wI3q6Sz35tU2gH+Bgh/TvDfWP/i02N82o9BUf55kLXipRGzFGFiIlr?= =?us-ascii?Q?0jHD/DEw8MvY8SNp6bCDIxvdjRudC4cCZz1pyO8b7Cp5udd0ms7UAyuFtL1f?= =?us-ascii?Q?jZaml2ZW0P7fpQstyI1v7Bdq0WzIqpcf8bnfWj0g4bhQGyXK3st4UZ7EoWvY?= =?us-ascii?Q?YMSFdusbrSDcuY0ygNuHP+VNP0szADLDk86Ojwv1fjb4EFn+w6JJ7eAi+cS7?= =?us-ascii?Q?oXS+kLliY9LdUa64hx69qzEZUlXvQYm5dGCglTh1uQ=3D=3D?= X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:PAXPR08MB6813.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(376005)(366007)(1800799015);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAWPR08MB10092 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AMS0EPF0000019B.eurprd05.prod.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 01eddeb7-f75f-4752-6ff4-08dc7b3c6ab1 X-Microsoft-Antispam-Message-Info: =?utf-8?B?SncxUlJlU3pwaERXa0lNbCtqTU1nUlJjVnVWdjZDK1FxK21kNklFSkRYT0dD?= =?utf-8?B?WVRldjBEQTNtdGd6NzY1dGVjYXZIMnZoOFllL2UydGxhbld3bzJrOU84Zm92?= =?utf-8?B?Z2dJMDFUTVowMG9QdWFzUEdVc2wyMlJBaWc3QUx5U0lTMzF4WmFBVzRBUHNO?= =?utf-8?B?K3hGVUZhOG90bGEvK29oVHJTREtrSjBvN3p4TnR6N0Iwd0V0ZnhONitXeklu?= =?utf-8?B?N1UvL09nZ0RETTFpZmtuYVpnSkN0K3lrQWpRT0FNbTFveWtiQW5IQzd3S3Rw?= =?utf-8?B?emNvNk9SQVo1NkFJMUxDSmlQQkFmd2F0cWJ0RUY3Z3F4M1A3MVg1T1A0blBZ?= =?utf-8?B?V2dyeFZoRjBlRVhWZCs2QnEyRVdoaG13WW9VNFNER2pVWnkwbHg0bmtEckUx?= =?utf-8?B?b2xzek55SElQRWovNXJQSUI4RlpZNVJmTHZkL3psSmVmYnE0V25nWUxnaFd0?= =?utf-8?B?VFNhbGN0ZlAvSVpkaFJkdXVqSDFCWWgzUUpyNDlWNmp1bG91RFhqVGlIelZk?= =?utf-8?B?MU9OZEp3ZTk4ZVBabW9URWdxSmhsOUJ1MDNiS0pZNzVLWkpFUzdHNWVzNUdB?= =?utf-8?B?SVJSSUZlVCtzQUZUd1JWSHcrVGFtb2EvcURxbndtVCtGVEVQWVEvdmFRb3hn?= =?utf-8?B?c0VWTFN6ZGx6SFh6T094Vm1CUkpjNjUzWTRXRlJkUVZxYThDNDlQWmdqS3NI?= =?utf-8?B?ZWNmOVAwNWRsVDRmTFBzdkR5Z2Ztbkhsby9EbXI1V2RtNGpkVFFkWVVPdGtY?= =?utf-8?B?NktZTERvaTZ5Z3lBZ3QxbGVTMUZtUFYzZ2RoMmQ0YXJtVmNHNnl0WGVYWUZ3?= =?utf-8?B?STI3VUIrR25sNnNadERBWHZQdlVpbGxzN1VScGloN00zMmp5UzQ0UEdkRTNF?= =?utf-8?B?eXhacEtMNUJQNjZRY3QxMTh4YkZKT2hNdzIybVp4UDdLZlozUFkxbEU1WTdk?= =?utf-8?B?VHF0TWZmYnZub3ZYUkE5WmYzUVlpT1Vab1pzdUxRVVdDT00vcGtDb2wvenBV?= =?utf-8?B?NDNQUExCUUVQYWdrQWkvY1hjbTJpTVFVVVppSTZPcGNuQUxDblZqaS8rSklZ?= =?utf-8?B?bTFzVlVSMjhsZWlxSFhUUXVKeWRvc1B0ZmRsM0J3WVlXVkxYSHRXWlVpY2la?= =?utf-8?B?ZlBYZW9xNjBNZmwyZDN6dGZTL3hmbU5HZ2NmZVRKOXRGdm5PZGh1Tm5MS05F?= =?utf-8?B?QmV3SCs3Vng5RHJHd0dRUno3RGRvdE56d3lEUkhIUVdvcFdkeVpiOG5jRWtj?= =?utf-8?B?Yld2RVRqOWNpNmZtdUxZTGdSNFN6MjdCeWRlZUt5cE55NTdlQ3ZQdlo1aGlK?= =?utf-8?B?ZFFTVTMrZGQzTDR3YjFYWGx4WjErQ1ppdTR0Vkg2ZnZhVHp3V2FEam92NDlY?= =?utf-8?B?RmRZWmF2VXhOZnRuZmVyTlpjZTBHQW9MejZTczJrbGZoQ3VNZ2Y0N3pFL0Jn?= =?utf-8?B?Y1BTNDFqMUVjOWhzNHlhVGpQb1Jhd3AvTWJDRG91VjNHWlhZT0xhdG9HQUx4?= =?utf-8?B?S1ZlNUlGRmZZZnh6OTdrREJrTDYrUDVhUG8rdThzek5qRUkwZUVmWDRkSWc0?= =?utf-8?B?OW9zRDBCSEgrR3BkT0EzTzdJTlNoU3gzVWxpZVFreGZmcWRTMVpqaktITjhJ?= =?utf-8?B?cU9PdE1Ca1FjRkxSNXNDdVpwS0N5QWR6dFAySjNVaWpQWnEzWUxWOGhVVXdV?= =?utf-8?B?SFJ2WlNYeXh6SWlUV2tBOHFVc3hzaUtWTDlnMHVQOEMzTk1WdW5EanNFKytC?= =?utf-8?B?Y1dVWGE2eTlBc2lWSVlGdU5uQmE4RWdpS2NOR2pDNkt3ampDYTQ5VHNuU0d0?= =?utf-8?B?VVVnMWlRd1ZGYWlaQ2xCYUkraVFMcVBQb05RZVBsRkN3b1pnNXBKcjArSThs?= =?utf-8?Q?F0Q5OMAdmFG7e?= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 15:24:29.0447 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 3cd9e9a4-f471-4e5e-738f-08dc7b3c703f X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AMS0EPF0000019B.eurprd05.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR08MB5528 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Thu, 23 May 2024 08:24:34 -0700 Resent-From: sami.mujawar@arm.com Reply-To: devel@edk2.groups.io,sami.mujawar@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: YLIOiCtpqzXdiFVhsS3rTx6Ux7686176AA= Content-Type: multipart/alternative; boundary="------------WRcnIwcPocY17wFIizKiIPKr" Content-Language: en-GB X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=nmoQa80V; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io --------------WRcnIwcPocY17wFIizKiIPKr Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Hi Sahil, Thank you for this patch. I have a minor suggession marked inline as [SAMI]. Otherwise this patch looks good to me. Reviewed-by: Sami Mujawar Regards, Sami Mujawar On 23/05/2024 11:55 am, Sahil Kaushal wrote: > From: sahil > > This patch adds error_handler1 and error_handler2 labels in > NorFlashCreateInstance() function to handle the cleanup. > > error_handler1: Frees just the Instance structure as the > ShadowBuffer is not allocated yet. > > error_handler2: Frees both Instance and Instance->ShadowBuffer. > > Signed-off-by: sahil > --- > Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c | 18 ++++++++++= +++----- > Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 19 ++++++++++= ++++----- > 2 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c b/Platform/AR= M/Drivers/NorFlashDxe/NorFlashDxe.c > index e01b05d91978..fd47bd9e4c63 100644 > --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c > +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c > @@ -135,7 +135,8 @@ NorFlashCreateInstance ( > =20 > > Instance->ShadowBuffer =3D AllocateRuntimePool (BlockSize); > > if (Instance->ShadowBuffer =3D=3D NULL) { > > - return EFI_OUT_OF_RESOURCES; > > + Status =3D EFI_OUT_OF_RESOURCES; > > + goto error_handler1; > > } > > =20 > > if (SupportFvb) { > > @@ -152,8 +153,7 @@ NorFlashCreateInstance ( > NULL > > ); > > if (EFI_ERROR (Status)) { > > - FreePool (Instance); > > - return Status; > > + goto error_handler2; > > } > > } else { > > Status =3D gBS->InstallMultipleProtocolInterfaces ( > > @@ -167,12 +167,20 @@ NorFlashCreateInstance ( > NULL > > ); > > if (EFI_ERROR (Status)) { > > - FreePool (Instance); > > - return Status; > > + goto error_handler2; > > } > > } > > =20 > > *NorFlashInstance =3D Instance; [SNIP] > + return EFI_SUCCESS; > > + > +error_handler1: > > + FreePool (Instance); > > + return Status; > > + > > +error_handler2: > > + FreePool (Instance->ShadowBuffer); > > + FreePool (Instance); > > return Status; [/SNIP] [SAMI] I think the above code can be simplified as below: --- + return Status; + +error_handler2: + FreePool (Instance->ShadowBuffer); +error_handler2: + FreePool (Instance); return Status; --- A similar change is reuired later in this patch below. If you agree, I will fix this up before merging the patch. [/SAMI] > } > > =20 > > diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/Pl= atform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c > index 16fe3762e125..17dfe26627dd 100644 > --- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c > +++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c > @@ -129,7 +129,8 @@ NorFlashCreateInstance ( > =20 > > Instance->ShadowBuffer =3D AllocateRuntimePool (BlockSize); > > if (Instance->ShadowBuffer =3D=3D NULL) { > > - return EFI_OUT_OF_RESOURCES; > > + Status =3D EFI_OUT_OF_RESOURCES; > > + goto error_handler1; > > } > > =20 > > if (SupportFvb) { > > @@ -142,16 +143,24 @@ NorFlashCreateInstance ( > &Instance->FvbProtocol > > ); > > if (EFI_ERROR (Status)) { > > - FreePool (Instance); > > - return Status; > > + goto error_handler2; > > } > > } else { > > DEBUG ((DEBUG_ERROR, "standalone MM NOR Flash driver only support F= VB.\n")); > > - FreePool (Instance); > > - return EFI_UNSUPPORTED; > > + Status =3D EFI_UNSUPPORTED; > > + goto error_handler2; > > } > > =20 > > *NorFlashInstance =3D Instance; > > + return EFI_SUCCESS; > > + > > +error_handler1: > > + FreePool (Instance); > > + return Status; > > + > > +error_handler2: > > + FreePool (Instance->ShadowBuffer); > > + FreePool (Instance); > > return Status; > > } > > =20 > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119164): https://edk2.groups.io/g/devel/message/119164 Mute This Topic: https://groups.io/mt/106260149/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --------------WRcnIwcPocY17wFIizKiIPKr Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Hi Sahil,

Thank you for this patch.

I have a minor suggession marked inline as [SAMI].

Otherwise this patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/05/2024 11:55 am, Sahil Kaushal wrote:
From: sahil <sahil@arm.com>

This patch adds error_handler1 and error_handler2 labels in
NorFlashCreateInstance() function to handle the cleanup.

error_handler1: Frees just the Instance structure as the
ShadowBuffer is not allocated yet.

error_handler2: Frees both Instance and Instance->ShadowBuffer.

Signed-off-by: sahil <sahil@arm.com>
---
 Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c          | 18 +++++++++++++=
-----
 Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 19 +++++++++++++=
+-----
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c b/Platform/ARM/=
Drivers/NorFlashDxe/NorFlashDxe.c
index e01b05d91978..fd47bd9e4c63 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -135,7 +135,8 @@ NorFlashCreateInstance (
=20

   Instance->ShadowBuffer =3D AllocateRuntimePool (BlockSize);

   if (Instance->ShadowBuffer =3D=3D NULL) {

-    return EFI_OUT_OF_RESOURCES;

+    Status =3D EFI_OUT_OF_RESOURCES;

+    goto error_handler1;

   }

=20

   if (SupportFvb) {

@@ -152,8 +153,7 @@ NorFlashCreateInstance (
                     NULL

                     );

     if (EFI_ERROR (Status)) {

-      FreePool (Instance);

-      return Status;

+      goto error_handler2;

     }

   } else {

     Status =3D gBS->InstallMultipleProtocolInterfaces (

@@ -167,12 +167,20 @@ NorFlashCreateInstance (
                     NULL

                     );

     if (EFI_ERROR (Status)) {

-      FreePool (Instance);

-      return Status;

+      goto error_handler2;

     }

   }

=20

   *NorFlashInstance =3D Instance;
[SNIP] 
+  return EFI_SUCCESS;

+

+error_handler1:

+  FreePool (Instance);

+  return Status;

+

+error_handler2:

+  FreePool (Instance->ShadowBuffer);

+  FreePool (Instance);

   return Status;

[/SNIP]

[SAMI] I think the above code can be simplified as below:

---

+ return Status;
+

+error_handler2:
+  FreePool (Instance->Shadow=
Buffer);

+error_handler2:

+  FreePool (Instance);

   return Status;
---

A similar change is reuired later in this patch below.

If you agree, I will fix this up before merging the patch.

[/SAMI]


 }

=20

diff --git a/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/Plat=
form/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
index 16fe3762e125..17dfe26627dd 100644
--- a/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
+++ b/Platform/ARM/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
@@ -129,7 +129,8 @@ NorFlashCreateInstance (
=20

   Instance->ShadowBuffer =3D AllocateRuntimePool (BlockSize);

   if (Instance->ShadowBuffer =3D=3D NULL) {

-    return EFI_OUT_OF_RESOURCES;

+    Status =3D EFI_OUT_OF_RESOURCES;

+    goto error_handler1;

   }

=20

   if (SupportFvb) {

@@ -142,16 +143,24 @@ NorFlashCreateInstance (
                       &Instance->FvbProtocol

                       );

     if (EFI_ERROR (Status)) {

-      FreePool (Instance);

-      return Status;

+      goto error_handler2;

     }

   } else {

     DEBUG ((DEBUG_ERROR, "standalone MM NOR Flash driver only support=
 FVB.\n"));

-    FreePool (Instance);

-    return EFI_UNSUPPORTED;

+    Status =3D EFI_UNSUPPORTED;

+    goto error_handler2;

   }

=20

   *NorFlashInstance =3D Instance;

+  return EFI_SUCCESS;

+

+error_handler1:

+  FreePool (Instance);

+  return Status;

+

+error_handler2:

+  FreePool (Instance->ShadowBuffer);

+  FreePool (Instance);

   return Status;

 }

=20
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#119164) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------WRcnIwcPocY17wFIizKiIPKr--