On 10/11/22 02:49, AKASHI Takahiro wrote:
The commit message is not accurate.
On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
The CloseProtocol() boot service requires a handle as first argument. Passing the protocol interface is incorrect.
Correct, but
CloseProtocol() only has an effect if called with a non-zero value for agent_handle. HandleProtocol() uses an opaque agent_handle when invoking OpenProtocol() (currently NULL).
No. OpenProtocol() is called with efi_root as an agent handle. So, calling CloseProtocol() is a right thing at the end.
Typically an agent handle is used to relate to a driver exposing the driver binding protocol. The root node does not expose the driver binding protocol.
Why would you want to create an open protocol information entry here?
Do you think anything with the code after the patch is wrong?
Best regards
Heinrich
Therefore HandleProtocol() should be avoided.
- Replace the LocateHandle() call by efi_search_protocol().
LocateHandle() -> efi_handle_protocol()
So you could have fixed this way: EFI_CALL(efi_close_protocol(handle, ..., &efi_root, NULL);
I preferred to use EFI_CALL() over this file as you can see.
-Takahiro Akashi
- Remove the CloseProtocol() call.
Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/efi_loader/efi_capsule.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index b6bd2d6af8..397e393a18 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance, efi_status_t ret;
for (i = 0, handle = handles; i < no_handles; i++, handle++) {
ret = EFI_CALL(efi_handle_protocol(*handle,&efi_guid_firmware_management_protocol,(void **)&fmp));
struct efi_handler *fmp_handler;ret = efi_search_protocol(*handle, &efi_guid_firmware_management_protocol,&fmp_handler);if (ret != EFI_SUCCESS) continue;
fmp = fmp_handler->protocol_interface;/* get device's image info */ info_size = 0;
@@ -215,10 +217,6 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64 instance, skip: efi_free_pool(package_version_name); free(image_info);
EFI_CALL(efi_close_protocol((efi_handle_t)fmp,&efi_guid_firmware_management_protocol,NULL, NULL));-- 2.37.2