From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 125A221CF58BA for ; Wed, 4 Oct 2017 08:03:04 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 33989C047B60; Wed, 4 Oct 2017 15:06:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 33989C047B60 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-111.rdu2.redhat.com [10.10.120.111]) by smtp.corp.redhat.com (Postfix) with ESMTP id 51A4161345; Wed, 4 Oct 2017 15:06:23 +0000 (UTC) To: "Zeng, Star" , Ard Biesheuvel Cc: "Ni, Ruiyu" , "Dong, Eric" , "edk2-devel@lists.01.org" , "leif.lindholm@linaro.org" , "Yao, Jiewen" References: <20171003171727.5641-1-ard.biesheuvel@linaro.org> <0C09AFA07DD0434D9E2A0C6AEB0483103B97E276@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9CE19A@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B97E44C@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B97E486@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: Date: Wed, 4 Oct 2017 17:06:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B97E486@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 04 Oct 2017 15:06:25 +0000 (UTC) Subject: Re: [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Oct 2017 15:03:04 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Star, On 10/04/17 16:09, Zeng, Star wrote: > Thanks for confirming the urgency. > > I have no strong motivation to keep/remove the ASSERT, I would like Ruiyu to argue and make the decision. > I mainly want the issue (the code ends up calling this function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext") could be root caused. it might be interesting to find out about the exact call stack. However, I'd like to point out that the exact purpose of the EfiBootManagerIsValidLoadOptionVariableName() function is to check *whether* the variable name is a valid boot option name or not. If not -- for whatever reason -- then it shouldn't ASSERT(); it should just return FALSE. Perhaps it's relevant: the function was made public in commit 3dc5c1ae5c757. Thanks Laszlo > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Wednesday, October 4, 2017 9:54 PM > To: Zeng, Star > Cc: Yao, Jiewen ; Ni, Ruiyu ; edk2-devel@lists.01.org; Dong, Eric ; leif.lindholm@linaro.org > Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname > > On 4 October 2017 at 14:49, Zeng, Star wrote: >> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it >> will be rejected by >> MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that will check the VariableName against UEFI spec “Table 13. Global Variables” >> if the VendorGuid is gEfiGlobalVariableGuid. >> >> >> >> I would suspect there is a bug at other place if the code ends up >> calling this function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext". >> > > That still does not mean you should ASSERT() here. The state of the variable store != the internals of the code, and so it should be considered external input to some extent. ASSERTs are meant to catch programming errors, not errors in the varstore image. > > >> >> Ard, >> >> Is the fix urgent or not for you? >> > > Not really. But fwupdate is shipping as part of many distros, so I guess others may run into it as well. > >> I may want to wait for Ruiyu’s back to take some look at the detail of it. >> > > That is fine. > >> At the same time, you may help check the code flow in some detail if >> you have free time, I think that will be helpful. J >> > > OK. > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >