From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 27949C4167B for ; Tue, 20 Dec 2022 06:45:53 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0672483D10; Tue, 20 Dec 2022 07:45:51 +0100 (CET) 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="DsVLkpgR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 43C2B84F87; Tue, 20 Dec 2022 07:45:49 +0100 (CET) Received: from mail-ed1-x533.google.com (mail-ed1-x533.google.com [IPv6:2a00:1450:4864:20::533]) (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 BDB1C839D5 for ; Tue, 20 Dec 2022 07:45:46 +0100 (CET) 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-ed1-x533.google.com with SMTP id s5so16070931edc.12 for ; Mon, 19 Dec 2022 22:45:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=7JUTIgg4PtjnjuqH+raF1gwS1l2a4YwmBpAUpoIlY6E=; b=DsVLkpgRGy06MB91VhQNcTl2lLH3VmYw6OFrf9q0AlUh90RciF4JprFNDsYeyW7a0o GANuvvAlVfe7fWekv1N7VNSXEjHP0RaRTewuj7iTu7qqSYsZaGLLBKnv/7hu3qHS1iGW qfcbBLd6GNxpG+O/83HiwFpt9lWhCtS99ynyMSmRtL9xP1eegN6tQz45YpLrp+v2TE3J eRvl/mpziiW0044hL4ejw7rEQYUsE9+/b7DMuyENLtS7j0X6g64CKOchRCI7916E2/on aQopU2kTETqMPm8INkRLg8u80Pe3Cxd37r8bkXbjTfFgj0krzvyG+kP6YTdNCXZUsTfd VzxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=7JUTIgg4PtjnjuqH+raF1gwS1l2a4YwmBpAUpoIlY6E=; b=T70BZosL5bvqGJOWNEND7U3I4gFtbUXFtV2BH+PKUqh5BSuGdU3XO1YDD7DcYjDj8v EWwYy5GFA8Oys1aQ6PgvGSvjxhPo2h1xkig10tLxBgUG1s3TLYX/ZBWRSiqaOi60TvTN WgrGZZiPNSfhBRIJKlskgGO5yb76PwjUjPlRM79Ef5pIQpH0D6sPIrWmiHNLK9tF8bKl kWuJ+ziBct+CUt/7pSbdJwnspcdjVpmDYweyziF7+qc+eX+jFNE/2CqY6ef31TijL40g XJmk6nLrlDfnxyCnoaBxjFYPC68sNgdMzl1O1euocH93QWK8waEIbCidFard1OUgLR+/ Ynkw== X-Gm-Message-State: AFqh2kpGr+SXgqkKxfq//8Dn1DtZF2JMXJl42gcPv+bznGF7zmM4tnYk wg50qgDg6Vv+zd4jRJhdKcrXCw== X-Google-Smtp-Source: AMrXdXthBG9WqIttlTwjvi4xIvhg2Vp8PH1hykVjHwipPScKBc01zz65Urb4Y0G2RbZ1gwiEhaBG3w== X-Received: by 2002:aa7:d3d9:0:b0:46b:1715:e64 with SMTP id o25-20020aa7d3d9000000b0046b17150e64mr821213edr.18.1671518746321; Mon, 19 Dec 2022 22:45:46 -0800 (PST) Received: from hera (ppp079167090036.access.hol.gr. [79.167.90.36]) by smtp.gmail.com with ESMTPSA id m11-20020a50ef0b000000b00472f9fd8bc6sm5177253eds.62.2022.12.19.22.45.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Dec 2022 22:45:45 -0800 (PST) Date: Tue, 20 Dec 2022 08:45:43 +0200 From: Ilias Apalodimas To: Masahisa Kojima Cc: u-boot@lists.denx.de, Heinrich Schuchardt Subject: Re: [PATCH v2 2/2] eficonfig: avoid SetVariable between GetNextVariableName calls Message-ID: References: <20221219023314.23959-1-masahisa.kojima@linaro.org> <20221219023314.23959-3-masahisa.kojima@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20221219023314.23959-3-masahisa.kojima@linaro.org> 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.6 at phobos.denx.de X-Virus-Status: Clean On Mon, Dec 19, 2022 at 11:33:13AM +0900, Masahisa Kojima wrote: > The current code calls efi_set_variable_int() to delete the > invalid boot option between calls to efi_get_next_variable_name_int(), > it may produce unpredictable results. > > This commit moves removal of the invalid boot option outside > of the efi_get_next_variable_name_int() calls. > EFI_NOT_FOUND returned from efi_get_next_variable_name_int() > indicates we retrieved all EFI variables, it should be treated > as EFI_SUCEESS. > > To address the checkpatch warning of too many leading tabs, > combine two if statement into one. > > Signed-off-by: Masahisa Kojima > --- > Changes in v2: > - fix typos > - use '!guidcmp()' instead of 'guidcmp() == 0' > - remove superfluous malloc() branch > > cmd/eficonfig.c | 54 ++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 38 insertions(+), 16 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index 0b07dfc958..ce7175a566 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -2310,13 +2310,14 @@ out: > efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_option *opt, > efi_status_t count) > { > - u32 i; > efi_uintn_t size; > void *load_option; > + u32 i, list_size = 0; > struct efi_load_option lo; > u16 *var_name16 = NULL; > u16 varname[] = u"Boot####"; > efi_status_t ret = EFI_SUCCESS; > + u16 *delete_index_list = NULL, *p; > efi_uintn_t buf_size; > > buf_size = 128; > @@ -2331,8 +2332,14 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op > efi_uintn_t tmp; > > ret = efi_next_variable_name(&buf_size, &var_name16, &guid); > - if (ret == EFI_NOT_FOUND) > + if (ret == EFI_NOT_FOUND) { > + /* > + * EFI_NOT_FOUND indicates we retrieved all EFI variables. > + * This should be treated as success. > + */ > + ret = EFI_SUCCESS; > break; > + } > if (ret != EFI_SUCCESS) > goto out; > > @@ -2349,31 +2356,46 @@ efi_status_t eficonfig_delete_invalid_boot_option(struct eficonfig_media_boot_op > if (ret != EFI_SUCCESS) > goto next; > > - if (size >= sizeof(efi_guid_bootmenu_auto_generated)) { > - if (guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated) == 0) { > - for (i = 0; i < count; i++) { > - if (opt[i].size == tmp && > - memcmp(opt[i].lo, load_option, tmp) == 0) { > - opt[i].exist = true; > - break; > - } > + if (size >= sizeof(efi_guid_bootmenu_auto_generated) && > + !guidcmp(lo.optional_data, &efi_guid_bootmenu_auto_generated)) { > + for (i = 0; i < count; i++) { > + if (opt[i].size == tmp && > + memcmp(opt[i].lo, load_option, tmp) == 0) { > + opt[i].exist = true; > + break; > } > + } > > - if (i == count) { > - ret = delete_boot_option(i); > - if (ret != EFI_SUCCESS) { > - free(load_option); > - goto out; > - } > + /* > + * The entire list of variables must be retrieved by > + * efi_get_next_variable_name_int() before deleting the invalid > + * boot option, just save the index here. > + */ > + if (i == count) { > + p = realloc(delete_index_list, sizeof(u32) * > + (list_size + 1)); > + if (!p) { > + ret = EFI_OUT_OF_RESOURCES; > + goto out; > } > + delete_index_list = p; > + delete_index_list[list_size++] = index; > } > } > next: > free(load_option); > } > > + /* delete all invalid boot options */ > + for (i = 0; i < list_size; i++) { > + ret = delete_boot_option(delete_index_list[i]); > + if (ret != EFI_SUCCESS) > + goto out; > + } > + > out: > free(var_name16); > + free(delete_index_list); > > return ret; > } > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas