public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pedro Falcato" <pedro.falcato@gmail.com>
To: devel@edk2.groups.io
Cc: Savva Mitrofanov <savvamtr@gmail.com>,
	Pedro Falcato <pedro.falcato@gmail.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Zhiguang Liu <zhiguang.liu@intel.com>
Subject: [edk2-devel] [PATCH 1/2] MdePkg/BaseLib: Fix CRC16-ANSI calculation
Date: Thu, 30 Nov 2023 02:46:10 +0000	[thread overview]
Message-ID: <20231130024611.67135-2-pedro.falcato@gmail.com> (raw)
In-Reply-To: <20231130024611.67135-1-pedro.falcato@gmail.com>

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.falcato@gmail.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@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 (#111884): https://edk2.groups.io/g/devel/message/111884
Mute This Topic: https://groups.io/mt/102886793/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-11-30  2:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30  2:46 [edk2-devel] [PATCH 0/2] MdePkg: Fix CRC16-ANSI calculation Pedro Falcato
2023-11-30  2:46 ` Pedro Falcato [this message]
2023-11-30 19:32   ` [edk2-devel] [PATCH 1/2] MdePkg/BaseLib: " Michael D Kinney
2023-11-30  2:46 ` [edk2-devel] [PATCH 2/2] MdePkg/Test: Add google tests for BaseLib Pedro Falcato
2023-11-30 19:32   ` Michael D Kinney
2023-11-30 20:15     ` Pedro Falcato
2023-11-30 21:31       ` Michael D Kinney
2023-11-30 22:44         ` Pedro Falcato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231130024611.67135-2-pedro.falcato@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox