From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-002e3701.pphosted.com (mx0a-002e3701.pphosted.com [148.163.147.86]) by mx.groups.io with SMTP id smtpd.web11.21093.1660684209483560604 for ; Tue, 16 Aug 2022 14:10:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@hpe.com header.s=pps0720 header.b=i4K8GupV; spf=permerror, err=parse error for token &{10 18 %{ir}.%{v}.%{d}.spf.has.pphosted.com}: invalid domain name (domain: hpe.com, ip: 148.163.147.86, mailfrom: prvs=0227955ec0=brian.johnson@hpe.com) Received: from pps.filterd (m0148663.ppops.net [127.0.0.1]) by mx0a-002e3701.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 27GKnJhE031606; Tue, 16 Aug 2022 21:10:09 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hpe.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pps0720; bh=uPdiBMl8dg6rsccEqI70Y8thbBSEimmMW1KTcUr1sfg=; b=i4K8GupVBZ6t5JzJXX79+CAL9gtQRrNu7fdPpC7m1zy3u1HPO8xERgXH/ULsxKB9DH60 CH/2KS+ZOSpAilkQ9ajJB3P1Y4Cs9wWRK34sbvABoF5RhHDhEDYNoWuQoG6DMiSm6ffZ EZ/BK7Ywc7jH0j1Q1nlo/GWF/8sh5yRvEF0yk+iv4PfAPMBCxnk7T1H9Cd+PWFaCH/aU Fr6mTR6T/aoVdcHUXAojmpxgWWaqf4z9H7LrgtxUSFKWIK4lVG7frmn1ZXTqmzLa+JXX kqlISAf5Ossslgs5VMeOuKycGF9btMnjPtrPsIIwa9quyKJnLKZEOMgDMKtqyrr8qPVs Nw== Received: from p1lg14879.it.hpe.com (p1lg14879.it.hpe.com [16.230.97.200]) by mx0a-002e3701.pphosted.com (PPS) with ESMTPS id 3j0jkt852s-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 16 Aug 2022 21:10:08 +0000 Received: from p1wg14926.americas.hpqcorp.net (unknown [10.119.18.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by p1lg14879.it.hpe.com (Postfix) with ESMTPS id 0142FD2EA; Tue, 16 Aug 2022 21:10:07 +0000 (UTC) Received: from p1wg14924.americas.hpqcorp.net (10.119.18.113) by p1wg14926.americas.hpqcorp.net (10.119.18.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Tue, 16 Aug 2022 09:10:07 -1200 Received: from p1wg14921.americas.hpqcorp.net (16.230.19.124) by p1wg14924.americas.hpqcorp.net (10.119.18.113) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15 via Frontend Transport; Tue, 16 Aug 2022 09:10:07 -1200 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (192.58.206.38) by edge.it.hpe.com (16.230.19.124) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Tue, 16 Aug 2022 09:08:58 -1200 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VWe3S4v0voN3a5aqYuJdP40T+okOGYJxs0DD4vAzWBjqi59SPR7gpAY1banj/CWIJHQRmDyHv5p8TNaUARaSPErdWySINcQ06m21qpkwDzFMzbzmPd9/k3O5tk5P/zd/wJrjgUjR4jWLLEnPR8tfrM8CiviHAzq6oeGjfSmr7SW6PjnTWAVqGwNHuh8DCjQ0zlJkXIULFRbVq9mkcVVqfcrJsWs44MouHuQrLAJfS3l48Z5a1dQjrj9EXm8mUaI7cDcGX2/MYxuGwd1w9b77/zGIl6wIDF+hRvzhXtFsOImHGUaJE1oSxhdtDcCHiACUIjCQ8+38HgC3K+h18ofZMA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=tIZ09+rWOOcM5/XrxbTDeWUEgr2kNBCPNI7tC0NMSV4=; b=S/md4QYZoustWuGN0KCVhh1jEo1ZItjN/kMoZKcFyd1i12bhvzn+1zHeyCP4ZRWhm5Wb83XshzrDUXg7XC8yucVzhMnI4C1cPoWM81Qm2dNUpLpN3gbIM5RqN50uZwAodaRYBONpAnxrj+o+Ss53nXtppMbl/Qth6ShV6FWkn7+8n6xnnDKv0Ms4qGM9Da+4f60gqLMzlUljkGRWQ7EjB/TMRGZ2vhvIXXg4R+KK5fnltgHWpa1hsnJV2w3Ldqv18fXTCywX6+NCaidZFwQGDNyVFKlOVVADajQFhtf3RlWrIuCss92xCXg4w9AuE67u8nqb8IUw7RMAcY7KGmPA+A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=hpe.com; dmarc=pass action=none header.from=hpe.com; dkim=pass header.d=hpe.com; arc=none Received: from MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:303:1c0::21) by PH7PR84MB1837.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:510:153::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.24; Tue, 16 Aug 2022 21:08:56 +0000 Received: from MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM ([fe80::d12a:8f08:593f:1fb1]) by MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM ([fe80::d12a:8f08:593f:1fb1%6]) with mapi id 15.20.5504.028; Tue, 16 Aug 2022 21:08:56 +0000 Message-ID: <8a033ba8-967e-002d-2d39-6d19273403d2@hpe.com> Date: Tue, 16 Aug 2022 16:08:53 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [edk2-devel] [PATCH 1/2] OvmfPkg: Introduce NULL class library to inhibit driver load To: , , Ard Biesheuvel CC: Yuan Yu , Gerd Hoffmann , "Pawel Polawski" , Oliver Steffen , "Jiewen Yao" References: <20220815094030.465587-1-ardb@kernel.org> <20220815094030.465587-2-ardb@kernel.org> <3cc22b45-149b-15c5-257d-347d1a13cd96@redhat.com> From: "Brian J. Johnson" Organization: HPE In-Reply-To: <3cc22b45-149b-15c5-257d-347d1a13cd96@redhat.com> X-ClientProxiedBy: DM6PR11CA0026.namprd11.prod.outlook.com (2603:10b6:5:190::39) To MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM (2603:10b6:303:1c0::21) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 401aad0b-fd30-4dd0-af9a-08da7fcb883e X-MS-TrafficTypeDiagnostic: PH7PR84MB1837:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: I3OV89EmQiO+WVxdsKKijcc5Z0JXjockyoHOqjk4dSazn+yInDlqItooNqwGD9pSQuRjMuXMOjhmpNj/tfJ5bCLf1NhlrCcIveGhE2z9ai67Lj4rMgZmtcVboe+fZWq9nSfi1qJFFcvQf2W+/5NoS+LY8YIoWsL7J6CkLqZxINYW+uoqTarJ170BgQdyrRg39wEb38/uB5tMfEXYokBtNtIyuO6zEWmzx28VixAH/nMZ2bPqaKYrfXc9HQ+em0YdIbzf2rNaBOE6/MHM0QlmXTog3l/T7eVmq0Zrez6+payg8DcA04eHZuVYZFWZ3Tj5HXaL1ZBL0Ws2pEZoJvdQZILz8Gl5Nx3/J7bebgcv5Wg0bXWQYAULKd2ZA7u1I6AN6Gj+Xg7I7AnRIr9c+U/MEkAW1dgs4Ovz4/IAQkOlxEr8S2XWy6d877RIFC/0FFGAKUurgHPTos51g8HwMntbs/xGKIO1/7u2fNC5L+lTYWtgNUoF6vzFZclQrJWvGWO8oAK0Or5BU4HL11QiyUx1xdgj2eMAM6pUwdDABIZYL9T5HoBWGV7v1fH3zkUC7wx5nRxLeSPB8LmSB7LkHjU2DJ6KlkXmMcUJVLV8g9AEyHFgXq+GnNs8gEXPITIayheWJYRGUmYgeiRfLB8xJKuYepoeyL1LO1kkXYAd8ICJGyoWzxK7vYj79BBFfHfLGXcad6iFbdbkrflBE9JSIfMFEWTQKVcsFkW2uWP3tt3ZYWEcRwjkwTJE1W2be0sbRG/CAJVPvrGj6LNwBTttHJs+4tCPgPvx9ELnPKPEn0/BZgXo3KmhZV6Pv1GRY62qCkbMsnVKu/876T290RyXyW1wW8weq0f+wVm64Y8+GuVmGVT1vOz9Cr4VF9AjSEMhUtg0D9brYpew8IFORMXjAwa+GQ== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM;PTR:;CAT:NONE;SFS:(13230016)(39860400002)(346002)(376002)(396003)(136003)(366004)(30864003)(38100700002)(26005)(82960400001)(31696002)(6506007)(36916002)(966005)(53546011)(6512007)(8936002)(478600001)(86362001)(5660300002)(6486002)(6666004)(41300700001)(4326008)(2906002)(66476007)(8676002)(2616005)(6916009)(66556008)(316002)(31686004)(186003)(66946007)(54906003)(83380400001)(36756003)(45980500001)(43740500002);DIR:OUT;SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SlE4UmNCSWRaUi9ZNVRLOTJIYVdUdllrTXNTYzBnem1TVklWVC9Bald3cXBR?= =?utf-8?B?N2lPa1pYMGp1UTZ6Z3p6Q3EwNzNzRVB6QVJ2U0JHQzEyZFh5S1RDYjdlZDRx?= =?utf-8?B?Z1JBQ2RRc1dtZHI5bnNmUnZXdnFQVTBIdTdSb3FDek5ndEFmbitOQUNJZ3Ju?= =?utf-8?B?WFdIME81QWhILzRFR2lFemRJNUgwN0s3ZUN5VzdvVTdSYSsxekdqWWlvb1VM?= =?utf-8?B?L3lqeGFsMG1FN0U0N1dMNzdXRS8yRm80dnk3QzFSOXBKc1BmZThUNVZLenEv?= =?utf-8?B?a2tRQVB0OGpFLzhVYk1kR1Z5STVQNGN3clhtcERGTmZaeGtmRlB2UTlsQzJj?= =?utf-8?B?QS9HcDY4cFJSR0pzZ3dxc0VQWEJ6TXQ3dWJrTUVGRUl1T0x0cDZOY0d0Y0k5?= =?utf-8?B?VSsxd1RFQk9hNGNWWmVIbkE3eXc4WUNnTTZMZU9wWEgzS1BVdUYzcHFRTmtB?= =?utf-8?B?ZzI2Z0tMMVB5VG5tcW91bFdTT2ExbTkrNUV3b3FCMlc4WUJzdGUzTk96RG1O?= =?utf-8?B?azQxNERyczM2Sm45amtZYjIrRlEySlFMRHUzOUdRYm8wL0NxWWhBN0txWUEy?= =?utf-8?B?cWR4VVR5aWlhanE4NlJGcGlkUk9CZlBHcGN5a0g4MEdoVjBmRjYyd1hiN3Js?= =?utf-8?B?RjBBR1ZSaFVmdlNxZzVuZDE5eWVWd3B2dmVYUXM2d0tuSFpndWcwVkpwdHFV?= =?utf-8?B?YjBIOWVWYkxFYTdGVk1UTGF6Ujd1ZjdRaXNqYjhpaDBOb2pqUjNaRGt2eDFD?= =?utf-8?B?ek4wcTBLZE5zMVJpZ3lGUmZHbjlEUW13OGd1RHBVWFh0MVZCVkxnaS9pVlk1?= =?utf-8?B?blpZSXhjcm1ZdDZ0QmhHQ1lnb3pzQjlBVk9ORUJJWGx4YVZJZE53R2xVVktP?= =?utf-8?B?QnJWQjI2WmZLdjNoUnIvZ0xWTDR5WUtKM2E0RmEyd3NWV01UeHZtNUEzZGxp?= =?utf-8?B?OHdNR1Q1VGljOW13Uzl2b2x4T0I0eG9CVWRDMmtqclVnamFxRXowZS9qb1Vz?= =?utf-8?B?RjA0ZHJXNnE0allseG9QRWxLWXJBbnFNUUtXMkp6dmxUY2N1Qnh1MWNpdkM3?= =?utf-8?B?emRYU1BpMlE4ZExkVTgzTzQrQm5UQlNSbHA0VUNIcUZMWlZMaTFqT2JORXJ4?= =?utf-8?B?aHc2LzBHTmV2R3hQWGxCZFo1bmhQR1dEWWRTRFB4QUN6SHBsVzhFRGNmeUdY?= =?utf-8?B?bDFlN0g0dnR2TEQyOU1jNHRpWmJSaGNDQmwya3A2aU40bTRiQVQxYkk4MmJY?= =?utf-8?B?YzRwUHNtVUFDZ0Zpa3JkaVlxd3MySFN1Rkc1QkRkajRJVFdPbm9EWEhlZHFh?= =?utf-8?B?TEYwNXM2Q2lTYU1MeUxRdEl1alB0UnA3ZkJONGtIT3FSRGk1aE9HbzlJQVpG?= =?utf-8?B?encveU55Njl4OU1Zc3BTWGtEUE1ORi9KVlpCM3JGM3FOdkk1YkhUN2RmY21j?= =?utf-8?B?VlY0RHhPaGJDY2pldDhlSS93YmxWUTR4OXVRTWw1eTBCdmNKRzY4TSs2V0xq?= =?utf-8?B?UkNPd2cxRWgrbHdQUUJvcmJMVkhpWm4zbFlNRll4SXQ4emwrWnlnSnJ1dVNj?= =?utf-8?B?eGdreE5kMmxoM1NHU1JjOG1XYUZBQjNxeHpBWm9nYldtWk05Z051dExJek1Y?= =?utf-8?B?Q2tHUUUxczd5bkNudEROT2c2V0N5eWRXRDBacXVNQnh5QmdvazVUTS9nbnlR?= =?utf-8?B?WEs4em9qVlpQbEV5VW9tTFo1OGxmWHRHR1dMZkdjbHIvcWpqbm5vUzBzYVF0?= =?utf-8?B?T0cvcElSa2Vldks2RVhmUk1MQS9Yc3d4SWh2dFhKa0V3bW5WRFowaUdTcmZB?= =?utf-8?B?WDlmY2l5U0JUSEFVVjREemd1OEFCUVptQ1ZZTTVHaGxQSXdOTk1EUC9ZYk5w?= =?utf-8?B?NS91RkxrSTlYcUovbFBCRUJ3UUlUMGJoYnFrVzErQ2RBRkVrRWxBRmtPUjk4?= =?utf-8?B?VnFJS3NtMHNFUmpRTjhiN0RodGRlYUVCNzBoUkE2UzFzZDJvS1k3TDI1bUVS?= =?utf-8?B?c0VkakYrUi9IanNiZW9QajNQM0pkbUJMQVc1OW4vbjEzelBHc2JrZTJYZG5N?= =?utf-8?B?V1hLRjE5ZEwyOG9xMnVwQTNnWkNtMmdLbGZxK216cnM5c2xVV0lNRzhwZVY3?= =?utf-8?Q?Sfqlxlz/v46NDOCDyjIQZGyRX?= X-MS-Exchange-CrossTenant-Network-Message-Id: 401aad0b-fd30-4dd0-af9a-08da7fcb883e X-MS-Exchange-CrossTenant-AuthSource: MW5PR84MB1354.NAMPRD84.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Aug 2022 21:08:56.8338 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 1ZBUf9/mqaYVUUt3omT3e3Cd+Ty5mV+ZE63hJLAORYJFN4j73dOGiGn19uNjYTq+4reT+jlb299An0wTLSU3Dw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR84MB1837 X-OriginatorOrg: hpe.com X-Proofpoint-ORIG-GUID: orgcultBOH0oydaF1Nl1gGxsJRK_iYrc X-Proofpoint-GUID: orgcultBOH0oydaF1Nl1gGxsJRK_iYrc X-Proofpoint-UnRewURL: 3 URL's were un-rewritten MIME-Version: 1.0 X-HPE-SCL: -1 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-16_08,2022-08-16_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 malwarescore=0 priorityscore=1501 impostorscore=0 bulkscore=0 lowpriorityscore=0 mlxscore=0 suspectscore=0 phishscore=0 clxscore=1011 spamscore=0 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000 definitions=main-2208160077 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 8/16/22 07:30, Laszlo Ersek wrote: > On 08/15/22 11:40, Ard Biesheuvel wrote: >> Add a new library that can be incorporated into any driver built from >> source, and which permits loading of the driver to be inhibited based on >> the value of a QEMU fw_cfg boolean variable. This will be used in a >> subsequent patch to allow dispatch of the IPv6 and IPv6 network protocol > > (1) typo? (should be "IPv4 and IPv6" I think) > >> driver to be controlled from the QEMU command line. >> >> Signed-off-by: Ard Biesheuvel >> --- >> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c | 30 ++++++++++++++++++++ >> OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf | 28 ++++++++++++++++++ >> OvmfPkg/OvmfPkg.dec | 4 +++ >> 3 files changed, 62 insertions(+) >> >> diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c >> new file mode 100644 >> index 000000000000..dc8544bc38be >> --- /dev/null >> +++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.c >> @@ -0,0 +1,30 @@ >> +// @file >> +// Copyright (c) 2022, Google LLC. All rights reserved.
>> +// SPDX-License-Identifier: BSD-2-Clause-Patent >> +// >> + >> +#include >> + >> +#include >> +#include >> + >> +STATIC CHAR16 mExitData[] = L"Driver dispatch inhibited by QEMU fw_cfg variable."; >> + >> +EFI_STATUS >> +EFIAPI >> +DriverLoadInhibitorLibConstructor ( >> + IN EFI_HANDLE Handle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + RETURN_STATUS Status; >> + BOOLEAN Enabled; >> + >> + Status = QemuFwCfgParseBool (FixedPcdGetPtr (PcdDriverInhibitorFwCfgVarName), >> + &Enabled); >> + if (!RETURN_ERROR (Status) && !Enabled) { >> + return gBS->Exit (Handle, EFI_REQUEST_UNLOAD_IMAGE, sizeof mExitData, >> + mExitData); > > (2) Per UEFI spec, ExitData should be allocated with > gBS->AllocatePool(). > > (3) EFI_REQUEST_UNLOAD_IMAGE is from the PI spec; while not wrong, I > think it's strange to use here. EFI_ABORTED or something similar from > the UEFI spec would be a better fit IMO. > > (4) And then, the big problem: > > I agree that returning an error from the constructor would not be > beneficial, as it would cause an assertion to fail in the > ProcessLibraryConstructorList() function, in the generated "AutoGen.c" > file. > > However, calling gBS->Exit() from a constructor seems unsafe to me, with > regard to library destructors. > > Now, in the current case (considering patch#2), this unsafety is not > visible. That's because: > > (quoting ProcessLibraryConstructorList() and > ProcessLibraryDestructorList() from > "Build/OvmfX64/NOOPT_GCC5/X64/NetworkPkg/Ip4Dxe/Ip4Dxe/DEBUG/AutoGen.c", > from an earlier build on my machine anyway): > >> VOID >> EFIAPI >> ProcessLibraryConstructorList ( >> IN EFI_HANDLE ImageHandle, >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> EFI_STATUS Status; >> >> Status = PlatformDebugLibIoPortConstructor (); >> ASSERT_RETURN_ERROR (Status); >> >> Status = UefiBootServicesTableLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status = DevicePathLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status = UefiRuntimeServicesTableLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status = UefiLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status = UefiHiiServicesLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> Status = DpcLibConstructor (ImageHandle, SystemTable); >> ASSERT_EFI_ERROR (Status); >> >> } >> >> >> >> VOID >> EFIAPI >> ProcessLibraryDestructorList ( >> IN EFI_HANDLE ImageHandle, >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> >> } > > there is no library destruction to speak of -- none of the used library > instances have resources they need to release at destruction time. > > However, the general case looks problematic. The new library constructor > call would be inserted *somewhere* in ProcessLibraryConstructorList() -- > the insertion point is likely "mostly unspecified", as no library > instance depends on DriverLoadInhibitorLib, and DriverLoadInhibitorLib > seems to depend on relatively few lib classes too. Therefore, in theory > anyway, the new lib constructor could call gBS->Exit() somewhere in the > middle of ProcessLibraryConstructorList(), with only some of the library > constructors having been executed. > > Then the questions are > > - does gBS->Exit() call ProcessLibraryDestructorList() or not? > > - if it does not, that could lead to memory leaks. > > - If it does though, is ProcessLibraryDestructorList() smart enough to > call only those destructors whose constructors have previously run? > > - If not, it could call destructors on never-constructed data. > > Unfortunately, this looks really tough to figure out; testing it (with > some actual library destructors) could be easier. > > > FWIW, there are two call sites for ProcessLibraryDestructorList() (for > UEFI/DXE drivers anyway); both in > "MdePkg/Library/UefiDriverEntryPoint/DriverEntryPoint.c": > > - One is inside the _ModuleEntryPoint() function. > > This call is reached only when the function designated as ENTRY_POINT > in the driver's INF file returns (note, said function is not the > actual entry point function of the driver -- the actual entry point is > the _ModuleEntryPoint() function). > > When gBS->Exit() is called, the CoreExit() function > [MdeModulePkg/Core/Dxe/Image/Image.c] long-jumps back to > CoreStartImage(), and no part of the driver's _ModuleEntryPoint() is > again used. So the first ProcessLibraryDestructorList() call site, > namely the one in ModuleEntryPoint(), is not reached when gBS->Exit() > is called. > > - The other ProcessLibraryDestructorList() call site is in > _DriverUnloadHandler() > [MdePkg/Library/UefiDriverEntryPoint/DriverEntryPoint.c]. > > Now it's not easy at all to say whether gBS->Exit() utilizes this > function or not, when it unloads the image (because, per spec, > gBS->Exit() *is* responsible for unloading the image). > > However, we need not track that down right now, to see that the > proposed patch is unsafe in this aspect. That's because > _ModuleEntryPoint() does the following: > >> // >> // Call constructor for all libraries >> // >> ProcessLibraryConstructorList (ImageHandle, SystemTable); >> >> // >> // Install unload handler... >> // >> if (_gDriverUnloadImageCount != 0) { >> Status = gBS->HandleProtocol ( >> ImageHandle, >> &gEfiLoadedImageProtocolGuid, >> (VOID **)&LoadedImage >> ); >> ASSERT_EFI_ERROR (Status); >> LoadedImage->Unload = _DriverUnloadHandler; >> } > > In other words, even if CoreExit() might call Unload --> > _DriverUnloadHandler() --> ProcessLibraryDestructorList() somewhere, > _ModuleEntryPoint() sets "Unload" to "_DriverUnloadHandler" only > *after* ProcessLibraryConstructorList() returns. And the proposed > patch calls gBS->Exit() from *inside* ProcessLibraryConstructorList(), > that is, when "Unload" is not set yet. > > On physical machines, I've seen firmware options for disabling the IP > stack entirely; I wonder how those firmwares do it... I don't know how any physical machine handles that particular option. But one approach would be to add a GUID to the depex of the module you want to control, and install it only when you want the module to be dispatched. That's pretty straightforward, although it does result in "Driver %g was discovered but not loaded!!" messages from CoreDisplayDiscoveredNotDispatched() if sufficient debugging is enabled. Brian J. Johnson > Laszlo > > On 08/15/22 11:40, Ard Biesheuvel wrote: >> + } >> + return EFI_SUCCESS; >> +} >> diff --git a/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf >> new file mode 100644 >> index 000000000000..ed521d12d335 >> --- /dev/null >> +++ b/OvmfPkg/Library/DriverLoadInhibitorLib/DriverLoadInhibitorLib.inf >> @@ -0,0 +1,28 @@ >> +## @file >> +# Copyright (c) 2022, Google LLC. All rights reserved.
>> +# SPDX-License-Identifier: BSD-2-Clause-Patent >> +# >> +## >> + >> +[Defines] >> + INF_VERSION = 1.29 >> + BASE_NAME = DriverLoadInhibitorLib >> + FILE_GUID = af4c2c0b-f7ed-4d61-ad97-5953982c3531 >> + MODULE_TYPE = DXE_DRIVER >> + VERSION_STRING = 1.0 >> + LIBRARY_CLASS = NULL >> + CONSTRUCTOR = DriverLoadInhibitorLibConstructor >> + >> +[Sources] >> + DriverLoadInhibitorLib.c >> + >> +[LibraryClasses] >> + QemuFwCfgSimpleParserLib >> + UefiBootServicesTableLib >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + OvmfPkg/OvmfPkg.dec >> + >> +[FixedPcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index 5af76a540529..e9a22cab088c 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -399,6 +399,10 @@ [PcdsFixedAtBuild] >> ## The Tdx accept page size. 0x1000(4k),0x200000(2M) >> gUefiOvmfPkgTokenSpaceGuid.PcdTdxAcceptPageSize|0x200000|UINT32|0x65 >> >> + ## The QEMU fw_cfg variable that DriverLoadInhibitorLib will check to >> + # decide whether to abort dispatch of the driver it is linked into. >> + gUefiOvmfPkgTokenSpaceGuid.PcdDriverInhibitorFwCfgVarName|""|VOID*|0x68 >> + >> [PcdsDynamic, PcdsDynamicEx] >> gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 >> > > > > > > -- Brian -------------------------------------------------------------------- "You're the flavor packet in the Ramen noodle brick of life, Roy." -- Three Fellows