From patchwork Mon Jul 24 10:17:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilias Apalodimas X-Patchwork-Id: 705745 Delivered-To: patch@linaro.org Received: by 2002:a5d:60ca:0:b0:317:2194:b2bc with SMTP id x10csp1696402wrt; Mon, 24 Jul 2023 03:17:58 -0700 (PDT) X-Google-Smtp-Source: APBJJlGRkr6QSG5is5LpyKOpSk002gBK+AkZp7XgDdBUs5pQcXylsnMgtvbNzXJVzmIlrcQTr0qI X-Received: by 2002:adf:e94b:0:b0:315:a6a5:fe95 with SMTP id m11-20020adfe94b000000b00315a6a5fe95mr6014578wrn.52.1690193878379; Mon, 24 Jul 2023 03:17:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690193878; cv=none; d=google.com; s=arc-20160816; b=mijSr1GbnhJeKsRU0So9jMsmAVzktJAVHhzrv5YdQAiTwznRlhiv36kCsu9m0wzJZA XzVkF+ohr1hb5FpcDfQckukUDqpc+ikGoyfGfIEwLPPGPCkql7Gar/deCym8NXoGbeU7 0BV1kxPBOF+wiGub5nf3HzN+i+38OtddhedEwOOEo9lyEa14UimUbMXGtsoAp2uF7PCN H3GxejC0T5CeXdTbMDyn0dfx2IeL4nGvD9TpOCzXJCIm1s2sPplGgmYYt5LJu6OUvfrE SQZp/jmTq6oXFuSl8DT4rzQn2LykZu1I4ZLJn/jX6r+vqsD6ZitIRUPx6sOPkWCIAhN4 9RIg== 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=P45cN8yyueWxnE0JlMWtIrAA78tQdj9ZcrRZRrt+LVE=; fh=iF+sL963o6szLCJ3+6yQpWrxRCr/TQHiTeMvry3V3qU=; b=F8PffuQOcaHAweeuCZ4jfNcgFLrLVCl7VzjbJO5uPpt/MNAbuC6C4U/aEc584A95WN +WNJlZe8QU4k9NQvmGUXS55up++gBmajtcq1u6Zw9/DHNi+yv+MkOg0zZqZPr39XTYf+ fWPEbrAIlCZrI5JSDB+0aKsjCszQMDRA9P27pvEh5ctfHoG60oeCOcOeT94Ygjd+wOCg 3SbLs5hffZ+D0KqDJyGNe44lUhqUUP2pNA4aFKgISokuE/s5FWc1RYzP26/vSh5USr2D KRDpN+avvH8GkH0o+RY1qtFoMMezcNnRQyZeCYPQOClAt24JGootJ5IHeLxmszh2uYW5 K4tQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=tIMYVw9X; 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 w16-20020adfd4d0000000b003140f54d91csi4720162wrk.45.2023.07.24.03.17.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jul 2023 03:17:58 -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=tIMYVw9X; 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 7D33D8685E; Mon, 24 Jul 2023 12:17:57 +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="tIMYVw9X"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 206C38685E; Mon, 24 Jul 2023 12:17:43 +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-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) (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 8ED2E86853 for ; Mon, 24 Jul 2023 12:17:40 +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-x632.google.com with SMTP id a640c23a62f3a-992ace062f3so741116966b.2 for ; Mon, 24 Jul 2023 03:17:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690193860; x=1690798660; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=P45cN8yyueWxnE0JlMWtIrAA78tQdj9ZcrRZRrt+LVE=; b=tIMYVw9XuaQ9dOgHjsmipA0uWHWgXKbnHL9+8ohh6pUC6ZGUmIaYio9UsFaFzA5VfT z8l3nkBSub12jfYHOR4Zy+iv0lN29cfN5ppOXAAHLrvd5uGngczQkD6HwK12QO8qQlrV oPSpqjob3qZaqRta6rPBAxUVtbiCi94f8K/eAyGP4ShMoAfc0unfaA28P6zfIgDhewTe fm9dRkuvk320P+3iHl3LBHeT98j5LSIQyp8jkYXryGG1NBoQJK7/Z4ia78/S88KOdRvE Fmf7hEWwV5cbphhEiFpwo/R2Re7cOJk3D96F7h/o2xos7qc4Gj3iJHWPW6hxv8D5RPV9 znHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690193860; x=1690798660; 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=P45cN8yyueWxnE0JlMWtIrAA78tQdj9ZcrRZRrt+LVE=; b=Rg2WpnXBwd6ZJ7Emi1tBJrUjifeWyYx8zFANlIzdxvfua0E0JIo4x+dZqGxlMIQL3O udiTHLh3YlwXrz/T2OMrQomUpsIwdQIRDTnO7CjFQOXoDGaQKag0Ec9/szh4GO8O31aD HogG/QzUvYRTdXHVMqZaStc+5uSLjEgimhGO+FuhrapNwKO14tqh4YzcXLajiVpVr2NF Zj9Sl/Z5ZQ2BMJJKlaRmK/5eYq6L+sBBKZtbVh1LRJANt+Vwwjyb9JRYfvQNRW7zLsSJ r97V1/tM+HH/UKbOuYX6Uv1xuQrxuhpIB3TMWgKJqPRVywEOhcFxj/lnaZj4gzWLCcNo y1cQ== X-Gm-Message-State: ABy/qLbOKAfh8SCeXfFSs5wuWL0CBAn2Z1vuvwgGG4YXnNDU1PSGqz6+ 9qmCHwId79Ox36W9lwmAPOaCeqot3FrnVGmSRIKMbw== X-Received: by 2002:a17:907:75f7:b0:982:89b3:8650 with SMTP id jz23-20020a17090775f700b0098289b38650mr9825902ejc.64.1690193860061; Mon, 24 Jul 2023 03:17:40 -0700 (PDT) Received: from localhost.localdomain (ppp046103056065.access.hol.gr. [46.103.56.65]) by smtp.gmail.com with ESMTPSA id h26-20020a170906261a00b009875a6d28b0sm6517831ejc.51.2023.07.24.03.17.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Jul 2023 03:17:39 -0700 (PDT) From: Ilias Apalodimas To: u-boot@lists.denx.de Cc: Ilias Apalodimas , Heinrich Schuchardt Subject: [PATCH v2] efi_loader: make efi_delete_handle() follow the EFI spec Date: Mon, 24 Jul 2023 13:17:36 +0300 Message-Id: <20230724101736.21821-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 Reviewed-by: Heinrich Schuchardt --- Changes since v1: - only report errors internally in efi_delete_handle() cmd/bootefi.c | 21 ++++++--------------- include/efi_loader.h | 2 +- lib/efi_driver/efi_uclass.c | 12 +++++------- lib/efi_loader/efi_boottime.c | 20 ++++++++++++++------ lib/efi_loader/efi_disk.c | 6 +++++- 5 files changed, 31 insertions(+), 30 deletions(-) -- 2.40.1 diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 5c0afec1544d..f73d6eb0e2d8 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,12 @@ 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 (ret != EFI_SUCCESS) + efi_delete_handle(&image_obj->header); + else + ret = efi_delete_handle(&image_obj->header); return ret != EFI_SUCCESS; } diff --git a/include/efi_loader.h b/include/efi_loader.h index b5fa0fe01ded..3a64eb9c6672 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -629,7 +629,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..66a45e156d60 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,11 @@ 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); free(bp); return ret; } diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 052fe481e473..0e89c8505b11 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; } /** diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 28c8cdf7100e..8983b86886b3 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -708,6 +708,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 +728,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;