linux-ext4 - Question regarding the use of CRC32c for checksumming

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Jun 2023 21:17:10 +0100
From:   Pedro Falcato <pedro.falcato@...il.com>
To:     linux-ext4@...r.kernel.org, "Darrick J. Wong" <djwong@...nel.org>
Subject: Question regarding the use of CRC32c for checksumming

Hi,

(+CC the original author, Darrick)
I've been investigating (in the context of my EFI ext4 driver) why all
ext4 checksums appear inverted. After making sure my CRC32c
implementation was correct and up-to-par with other ones, I looked at
the fs/ext4 checksumming code, which took me to the implementation of
ext4_chksum in ext4.h (excuse the gmail whitespace damage):

>static inline u32 ext4_chksum(struct ext4_sb_info *sbi, u32 crc,
>       const void *address, unsigned int length)
>{
> struct {
> struct shash_desc shash;
> char ctx[4];
> } desc;

Open coding the crc32c crypto driver's internal state, seemingly to save a call?
>
> BUG_ON(crypto_shash_descsize(sbi->s_chksum_driver)!=sizeof(desc.ctx));
>
> desc.shash.tfm = sbi->s_chksum_driver;
> *(u32 *)desc.ctx = crc;

...we set the starting CRC
>
> BUG_ON(crypto_shash_update(&desc.shash, address, length));

then call update, which keeps the current internal state in ctx[4]
>
> return *(u32 *)desc.ctx;

and then we never call ->final() (nor ->finup()), which for crc32c would do:
> put_unaligned_le32(~ctx->crc, out);

and as such get me the properly "inverted" crc32c I would expect.
FreeBSD never found this issue as their calculate_crc32c seems borked
too, and never inverts the result.

Is my assessment correct? Was ->final() never called on purpose, or is
it an accident? Or is this merely a CRC32c variation I'm unaware of?

I'd like to make sure I get all the context on this, before sending
any kind of documentation patch :)

Thanks,
Pedro