Re: [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

[Laszlo Ersek <lersek@redhat.com>]

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 <star.zeng@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>; 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 <star.zeng@intel.com> 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
>