| Message ID | 20241020180258.8060-1-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: mips/crc32 - fix the CRC32C implementation | expand |
Very good catch.
Acked-by: Wentao Guan <guanwentao@uniontech.com>
On 2024/10/21 02:02, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Commit ca459e5f826f ("crypto: mips/crc32 - Clean up useless assignment > operations") changed crc32c_mips_le_hw() to use the instructions that > use the "regular" CRC32 polynomial instead of the Castagnoli polynomial. > Therefore it can't be computing CRC32C values correctly anymore. > > I haven't been successful in running a MIPS kernel in QEMU, but based on > code review this is the fix that is needed. > > Fixes: ca459e5f826f ("crypto: mips/crc32 - Clean up useless assignment operations") > Cc: Guan Wentao <guanwentao@uniontech.com> > Cc: WangYuli <wangyuli@uniontech.com> > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > > This is a regression in 6.12, so it should be fixed in a 6.12-rc. > > arch/mips/crypto/crc32-mips.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c > index a7a1d43a1b2ca..90eacf00cfc31 100644 > --- a/arch/mips/crypto/crc32-mips.c > +++ b/arch/mips/crypto/crc32-mips.c > @@ -121,24 +121,24 @@ static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len) > > if (IS_ENABLED(CONFIG_64BIT)) { > for (; len >= sizeof(u64); p += sizeof(u64), len -= sizeof(u64)) { > u64 value = get_unaligned_le64(p); > > - CRC32(crc, value, d); > + CRC32C(crc, value, d); > } > > if (len & sizeof(u32)) { > u32 value = get_unaligned_le32(p); > > - CRC32(crc, value, w); > + CRC32C(crc, value, w); > p += sizeof(u32); > } > } else { > for (; len >= sizeof(u32); len -= sizeof(u32)) { > u32 value = get_unaligned_le32(p); > > - CRC32(crc, value, w); > + CRC32C(crc, value, w); > p += sizeof(u32); > } > } > > if (len & sizeof(u16)) { > > base-commit: 7fa4be6d6752512278c4cbf2d2745568626e7369 Ah...I apologize for the oversight that introduced this bug...And it's indeed a necessary fix. Thanks, Acked-by: WangYuli <wangyuli@uniontech.com>
Eric Biggers <ebiggers@kernel.org> wrote: > From: Eric Biggers <ebiggers@google.com> > > Commit ca459e5f826f ("crypto: mips/crc32 - Clean up useless assignment > operations") changed crc32c_mips_le_hw() to use the instructions that > use the "regular" CRC32 polynomial instead of the Castagnoli polynomial. > Therefore it can't be computing CRC32C values correctly anymore. > > I haven't been successful in running a MIPS kernel in QEMU, but based on > code review this is the fix that is needed. > > Fixes: ca459e5f826f ("crypto: mips/crc32 - Clean up useless assignment operations") > Cc: Guan Wentao <guanwentao@uniontech.com> > Cc: WangYuli <wangyuli@uniontech.com> > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > > This is a regression in 6.12, so it should be fixed in a 6.12-rc. > > arch/mips/crypto/crc32-mips.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Patch applied. Thanks.
diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c index a7a1d43a1b2ca..90eacf00cfc31 100644 --- a/arch/mips/crypto/crc32-mips.c +++ b/arch/mips/crypto/crc32-mips.c @@ -121,24 +121,24 @@ static u32 crc32c_mips_le_hw(u32 crc_, const u8 *p, unsigned int len) if (IS_ENABLED(CONFIG_64BIT)) { for (; len >= sizeof(u64); p += sizeof(u64), len -= sizeof(u64)) { u64 value = get_unaligned_le64(p); - CRC32(crc, value, d); + CRC32C(crc, value, d); } if (len & sizeof(u32)) { u32 value = get_unaligned_le32(p); - CRC32(crc, value, w); + CRC32C(crc, value, w); p += sizeof(u32); } } else { for (; len >= sizeof(u32); len -= sizeof(u32)) { u32 value = get_unaligned_le32(p); - CRC32(crc, value, w); + CRC32C(crc, value, w); p += sizeof(u32); } } if (len & sizeof(u16)) {