From patchwork Mon Jun 26 10:04:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilias Apalodimas X-Patchwork-Id: 696433 Delivered-To: patch@linaro.org Received: by 2002:adf:e885:0:0:0:0:0 with SMTP id d5csp3135754wrm; Mon, 26 Jun 2023 03:04:53 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4nISY7HcLziAH5CjTfclFU29ayz2OQlAwnAmzFSOQADXpqeKI0ZYf+fEVsJ0xXmjlvPWTn X-Received: by 2002:ac2:5bca:0:b0:4f8:67e7:8a1c with SMTP id u10-20020ac25bca000000b004f867e78a1cmr7439237lfn.45.1687773893614; Mon, 26 Jun 2023 03:04:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687773893; cv=none; d=google.com; s=arc-20160816; b=DT6aGtswCYkOqxF+eSy/IUZtZG8e9hoyEaBrfCr3oBoFWYp5k8wpYnVtQyD890bG3l t3hW4IwNafvqzrBsborfcQrtrhmZJ/FRHCPiygoKAOOoaMJPiW2STq9SrD4F/pRu02Us a9lUks3A/t7QGlrdXSREEPeCkimZcOAViML0xQ+6zJDD3krqCVckfO6NiAKspt8DHWk7 qRWkSWqtiosGdXUSwQa5nieDICnhMhDmov8qRZB70f7kS1UAjxVhpfTdPREuXmQQt/HG VpaZ0shmm5wcfnopHSew0ToebRjIqVRlEOXlM2BX+JCjVSHR6gokYan7DPKrtmqQ9OaM vp7Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:cc:to:from:dkim-signature; bh=+IlRiNyE3EAA6Hq+s2IrXdM3GQeYdPe+RYyrxDmuNwc=; fh=dnCvuuGyHkzHtu4+LFxMpKl3VDB/FaBtn7IfZgSwGxw=; b=DGvthky5QiWWpR6WJjL65md7IPiEHSSZH+9G5KAidhfL0DJzZp2Ng4Recv9D5LVKLD EBgUKrOMESFpG/AxSUVNmqlK+DDYJpNeDouMgM9cQ93bA4C4dp9Y79RbMLvji5hTfNaM fWnlLraKaLy5/oNxKg02Pk3hb2uFEYFYR008muVcmRibR36y5ALamA4WhY/ABSfXAZyL jaYWNKHG8uShmnmSqHf4Iw89Qo+U7xtLaOOHpjdiLu4JWx03RHZIL7TRHDOlqnwibF6b Nwn+VXEt1faD4J5H4rEV56LiMullwMe3UTEnaloMC3tgsSuR7uFjQSjoSDSlIdz7VWHR h++Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cATcOJlf; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 85.214.62.61 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from phobos.denx.de (phobos.denx.de. [85.214.62.61]) by mx.google.com with ESMTPS id z21-20020ac25df5000000b004f768e31201si1446289lfq.400.2023.06.26.03.04.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Jun 2023 03:04:53 -0700 (PDT) Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de designates 85.214.62.61 as permitted sender) client-ip=85.214.62.61; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cATcOJlf; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 85.214.62.61 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 2E347846B5; Mon, 26 Jun 2023 12:04:50 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="cATcOJlf"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4819B86010; Mon, 26 Jun 2023 12:04:47 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com [IPv6:2a00:1450:4864:20::62f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id E04E880634 for ; Mon, 26 Jun 2023 12:04:43 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-987accb4349so487018366b.0 for ; Mon, 26 Jun 2023 03:04:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1687773883; x=1690365883; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=+IlRiNyE3EAA6Hq+s2IrXdM3GQeYdPe+RYyrxDmuNwc=; b=cATcOJlfVJXD0u3jQbSmn/LAhyi5WpZJn3VVa7kpM7t7UoxUwSaapVvyC/XIkE9Ebi 84ihHtvcSOFLsN+XesdOS694tOuyfdyK1wF8FiUwcCzVN0BIECxtXL+o8SK8Eht+EGIE lgvTH08HgHhcqCaW6W2CAmLXsXh6unJO8i4MCd12Q24nUao9YA77LI6oUuUGxRsaLdes +SOcbRi6pD78Sw5FYd5Cl8zxefw6U1+wqRGpv23HgUdxa22qcZFccb5XFr1H47G0JEf9 X+18K1nfIO+YVTkaacmVwZOuVnf/wqh+f969FWqFFxKJ/c+JgKYhpBTJZTVXJkfH5qzZ 6nBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687773883; x=1690365883; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+IlRiNyE3EAA6Hq+s2IrXdM3GQeYdPe+RYyrxDmuNwc=; b=LolvU5k8U6ZVMSdBUHDO8DS1XFMOO3SodkO6qiDPTVTD68KMFn3O0UD/o+9F1XsOIm uBwHtCzILa59d2nUXbvk6J3lwPwClyqWCdqrqFTyCiZmknpy6/rsCrNqd6I7lLjHCGEh MzeoYkeL/tV4L1rqEtbiGQm7GKn9swyVPavYVW1/BBSx+ohyOYodjP/ai9ktZtbJdi0g mJqw8RS8nvuh2VIVfTQazMf1PEWIsKvvggjXVa/0nkgxSJvDGKf3LlUy30fKzgqkdkT7 3PMrJ4mhgf9ElCtDfb0ts1qZWjWDPDJb1PmoOGAtBhilBHvjhg18vSlpfA8LyRO3JWvz 6DvQ== X-Gm-Message-State: AC+VfDzmH8R8QyrVYiaO+hwU2ZECDVGP9azRUiBQOJdvz+izSIh7of52 Cjn6zw0rZ50esEBL6wk0VElSeDx4jOqaAVtd7RasZw== X-Received: by 2002:a17:907:7d8a:b0:989:444b:725 with SMTP id oz10-20020a1709077d8a00b00989444b0725mr17317771ejc.26.1687773883474; Mon, 26 Jun 2023 03:04:43 -0700 (PDT) Received: from localhost.localdomain (ppp089210114029.access.hol.gr. [89.210.114.29]) by smtp.gmail.com with ESMTPSA id bm4-20020a170906c04400b0094f07545d40sm3041188ejb.220.2023.06.26.03.04.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Jun 2023 03:04:43 -0700 (PDT) From: Ilias Apalodimas To: u-boot@lists.denx.de Cc: Ilias Apalodimas , Heinrich Schuchardt Subject: [PATCH] efi_loader: make efi_delete_handle() follow the EFI spec Date: Mon, 26 Jun 2023 13:04:40 +0300 Message-Id: <20230626100440.107303-1-ilias.apalodimas@linaro.org> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean The EFI doesn't allow removal of handles, unless all hosted protocols are cleanly removed. Our efi_delete_handle() is a bit intrusive. Although it does try to delete protocols before removing a handle, it doesn't care if that fails. Instead it only returns an error if the handle is invalid. On top of that none of the callers of that function check the return code. So let's rewrite this in a way that fits the EFI spec better. Instead of forcing the handle removal, gracefully uninstall all the handle protocols. According to the EFI spec when the last protocol is removed the handle will be deleted. Also switch all the callers and check the return code. Some callers can't do anything useful apart from reporting an error. The disk related functions on the other hand, can prevent a medium that is being used by EFI from removal. The only function that doesn't check the result is efi_delete_image(). But that function needs a bigger rework anyway, so we can clean it up in the future Signed-off-by: Ilias Apalodimas --- Heinrich this needs to be applied on top of https://lore.kernel.org/u-boot/20230620061932.113292-1-ilias.apalodimas@linaro.org/ cmd/bootefi.c | 19 ++++--------------- include/efi_loader.h | 2 +- lib/efi_driver/efi_uclass.c | 13 ++++++------- lib/efi_loader/efi_boottime.c | 26 ++++++++++++++++++-------- lib/efi_loader/efi_disk.c | 17 +++++++++++++---- 5 files changed, 42 insertions(+), 35 deletions(-) -- 2.40.1 diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 8aa15a64c836..60b9e950ffdc 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -606,20 +606,6 @@ failure: return ret; } -/** - * bootefi_run_finish() - finish up after running an EFI test - * - * @loaded_image_info: Pointer to a struct which holds the loaded image info - * @image_obj: Pointer to a struct which holds the loaded image object - */ -static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, - struct efi_loaded_image *loaded_image_info) -{ - efi_restore_gd(); - free(loaded_image_info->load_options); - efi_delete_handle(&image_obj->header); -} - /** * do_efi_selftest() - execute EFI selftest * @@ -638,7 +624,10 @@ static int do_efi_selftest(void) /* Execute the test */ ret = EFI_CALL(efi_selftest(&image_obj->header, &systab)); - bootefi_run_finish(image_obj, loaded_image_info); + efi_restore_gd(); + free(loaded_image_info->load_options); + if (efi_delete_handle(&image_obj->header) != EFI_SUCCESS) + log_err("Failed to delete loaded image handle\n"); return ret != EFI_SUCCESS; } diff --git a/include/efi_loader.h b/include/efi_loader.h index e530bcf15f76..43ccf49abec4 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -618,7 +618,7 @@ void efi_add_handle(efi_handle_t obj); /* Create handle */ efi_status_t efi_create_handle(efi_handle_t *handle); /* Delete handle */ -void efi_delete_handle(efi_handle_t obj); +efi_status_t efi_delete_handle(efi_handle_t obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const efi_handle_t handle); /* Locate device_path handle */ diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index 45f935198874..d8755541fc14 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv) bp->ops = ops; ret = efi_create_handle(&bp->bp.driver_binding_handle); - if (ret != EFI_SUCCESS) { - free(bp); - goto out; - } + if (ret != EFI_SUCCESS) + goto err; bp->bp.image_handle = bp->bp.driver_binding_handle; ret = efi_add_protocol(bp->bp.driver_binding_handle, &efi_guid_driver_binding_protocol, bp); @@ -299,11 +297,12 @@ static efi_status_t efi_add_driver(struct driver *drv) if (ret != EFI_SUCCESS) goto err; } -out: - return ret; + return ret; err: - efi_delete_handle(bp->bp.driver_binding_handle); + if (bp->bp.driver_binding_handle && + efi_delete_handle(bp->bp.driver_binding_handle) != EFI_SUCCESS) + log_err("Failed to delete handle\n"); free(bp); return ret; } diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index d75a3336e3f1..5123055d7f5d 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -59,6 +59,10 @@ static efi_handle_t current_image; static volatile gd_t *efi_gd, *app_gd; #endif +static efi_status_t efi_uninstall_protocol + (efi_handle_t handle, const efi_guid_t *protocol, + void *protocol_interface); + /* 1 if inside U-Boot code, 0 if inside EFI payload code */ static int entry_count = 1; static int nesting_level; @@ -610,8 +614,8 @@ static efi_status_t efi_remove_all_protocols(const efi_handle_t handle) list_for_each_entry_safe(protocol, pos, &efiobj->protocols, link) { efi_status_t ret; - ret = efi_remove_protocol(handle, &protocol->guid, - protocol->protocol_interface); + ret = efi_uninstall_protocol(handle, &protocol->guid, + protocol->protocol_interface); if (ret != EFI_SUCCESS) return ret; } @@ -622,19 +626,23 @@ static efi_status_t efi_remove_all_protocols(const efi_handle_t handle) * efi_delete_handle() - delete handle * * @handle: handle to delete + * + * Return: status code */ -void efi_delete_handle(efi_handle_t handle) +efi_status_t efi_delete_handle(efi_handle_t handle) { efi_status_t ret; ret = efi_remove_all_protocols(handle); - if (ret == EFI_INVALID_PARAMETER) { - log_err("Can't remove invalid handle %p\n", handle); - return; + if (ret != EFI_SUCCESS) { + log_err("Handle %p has protocols installed. Unable to delete\n", handle); + return ret; } list_del(&handle->link); free(handle); + + return ret; } /** @@ -1816,7 +1824,8 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, return ret; failure: printf("ERROR: Failure to install protocols for loaded image\n"); - efi_delete_handle(&obj->header); + if (efi_delete_handle(&obj->header) != EFI_SUCCESS) + log_err("Failed to delete handle for loaded image\n"); free(info); return ret; } @@ -2097,7 +2106,8 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy, info->parent_handle = parent_image; } else { /* The image is invalid. Release all associated resources. */ - efi_delete_handle(*image_handle); + if (efi_delete_handle(*image_handle) != EFI_SUCCESS) + log_err("Failed to delete image handle\n"); *image_handle = NULL; free(info); } diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 28c8cdf7100e..fae440a79225 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -537,7 +537,8 @@ static efi_status_t efi_disk_add_dev( } return EFI_SUCCESS; error: - efi_delete_handle(&diskobj->header); + if (efi_delete_handle(&diskobj->header) != EFI_SUCCESS) + log_err("Failed to delete handle\n"); return ret; } @@ -577,7 +578,9 @@ static int efi_disk_create_raw(struct udevice *dev, efi_handle_t agent_handle) } if (efi_link_dev(&disk->header, dev)) { efi_free_pool(disk->dp); - efi_delete_handle(&disk->header); + ret = efi_delete_handle(&disk->header); + if (ret != EFI_SUCCESS) + log_err("Failed to delete handle\n"); return -EINVAL; } @@ -632,7 +635,9 @@ static int efi_disk_create_part(struct udevice *dev, efi_handle_t agent_handle) } if (efi_link_dev(&disk->header, dev)) { efi_free_pool(disk->dp); - efi_delete_handle(&disk->header); + ret = efi_delete_handle(&disk->header); + if (ret != EFI_SUCCESS) + log_err("Failed to delete handle\n"); return -1; } @@ -708,6 +713,7 @@ int efi_disk_remove(void *ctx, struct event *event) efi_handle_t handle; struct blk_desc *desc; struct efi_disk_obj *diskobj = NULL; + efi_status_t ret; if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) return 0; @@ -727,10 +733,13 @@ int efi_disk_remove(void *ctx, struct event *event) return 0; } + ret = efi_delete_handle(handle); + if (ret != EFI_SUCCESS) + return -1; + if (diskobj) efi_free_pool(diskobj->dp); - efi_delete_handle(handle); dev_tag_del(dev, DM_TAG_EFI); return 0;