Reviewed-by: Michael D Kinney <michael.d.kin...@intel.com> > -----Original Message----- > From: Pedro Falcato <pedro.falc...@gmail.com> > Sent: Wednesday, November 29, 2023 6:46 PM > To: devel@edk2.groups.io > Cc: Savva Mitrofanov <savva...@gmail.com>; Pedro Falcato > <pedro.falc...@gmail.com>; Gao, Liming <gaolim...@byosoft.com.cn>; Kinney, > Michael D <michael.d.kin...@intel.com>; Liu, Zhiguang > <zhiguang....@intel.com> > Subject: [PATCH 1/2] MdePkg/BaseLib: Fix CRC16-ANSI calculation > > The current CalculateCrc16Ansi implementation does the following: > 1) Invert the passed checksum > 2) Calculate the new checksum by going through data and using the > lookup table > 3) Invert it back again > > This emulated my design for CalculateCrc32c, where 0 is > passed as the initial checksum, and it inverts in the end. > However, CRC16 does not invert the checksum on input and output. > So this is incorrect. > > Fix the problem by not inverting input checksums nor output checksums. > Callers should now pass CRC16ANSI_INIT as the initial value instead of > "0". This is a breaking change. > > This problem was found out-of-list when older ext4 filesystems > (that use crc16 checksums) failed to mount with "corruption". > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4609 > Signed-off-by: Pedro Falcato <pedro.falc...@gmail.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Zhiguang Liu <zhiguang....@intel.com> > --- > MdePkg/Include/Library/BaseLib.h | 5 +++++ > MdePkg/Library/BaseLib/CheckSum.c | 4 ++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/MdePkg/Include/Library/BaseLib.h > b/MdePkg/Include/Library/BaseLib.h > index 5d7067ee854e..728e89d2bf44 100644 > --- a/MdePkg/Include/Library/BaseLib.h > +++ b/MdePkg/Include/Library/BaseLib.h > @@ -4599,6 +4599,11 @@ CalculateCrc16Ansi ( > IN UINT16 InitialValue > ); > > +// > +// Initial value for the CRC16-ANSI algorithm, when no prior checksum has > been calculated. > +// > +#define CRC16ANSI_INIT 0xffff > + > /** > Calculates the CRC32c checksum of the given buffer. > > diff --git a/MdePkg/Library/BaseLib/CheckSum.c > b/MdePkg/Library/BaseLib/CheckSum.c > index b6a076573191..57d324c82b22 100644 > --- a/MdePkg/Library/BaseLib/CheckSum.c > +++ b/MdePkg/Library/BaseLib/CheckSum.c > @@ -678,13 +678,13 @@ CalculateCrc16Ansi ( > > Buf = Buffer; > > - Crc = ~InitialValue; > + Crc = InitialValue; > > while (Length-- != 0) { > Crc = mCrc16LookupTable[(Crc & 0xFF) ^ *(Buf++)] ^ (Crc >> 8); > } > > - return ~Crc; > + return Crc; > } > > GLOBAL_REMOVE_IF_UNREFERENCED STATIC CONST UINT32 mCrc32cLookupTable[256] > = { > -- > 2.43.0
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111912): https://edk2.groups.io/g/devel/message/111912 Mute This Topic: https://groups.io/mt/102886793/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
