On 03/07/18 16:35, Zhang, Chao B wrote: > Star: > Why do we need to add HashInterfaceHob->SupportedHashMask = 0? > HashInterfaceHob is internally maintained and accessed by HashLibRouterPei. > There is no impact to leave the value after module has been re-shadowed.
There seems to be no functional requirement for clearing the SupportedHashMask field, except the original (buggy) ZeroMem() call looked like it *intended* to clear the field. So now that we have fixed the buffer overflow, we should decide whether we want to stick with the original intent (that would mean continuing to clear the field, one way or another), or to depart from the original intent -- but that would merit a comment in the code (or, it would have deserved a comment in the commit message). In short, it's not great to do two independent things in the same patch, namely (a) fix the buffer overflow, (b) silently diverge from the original intent, even if that's functionally justified. Such changes should be split to two patches, or else explained in detail. Thanks Laszlo _______________________________________________ edk2-devel mailing list firstname.lastname@example.org https://lists.01.org/mailman/listinfo/edk2-devel