| Message ID | 20170925190634.21738-1-tuomas.tynkkynen@iki.fi |
|---|---|
| State | Accepted |
| Commit | 385b73185596cfc9e2acb74ab66abe91c06177f3 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v2,1/4] fs/ext4: Fix group descriptor checksum calculation | expand |
On Mon, Sep 25, 2017 at 10:06:31PM +0300, Tuomas Tynkkynen wrote: > The current code doesn't compute the group descriptor checksum correctly > for the filesystems that e2fsprogs 1.43.4 creates (they have > 'Group descriptor size: 64' as reported by tune2fs). Extend the checksum > calculation to be done as ext4_group_desc_csum() does in Linux. > > This fixes these errors in dmesg from running fs-test.sh and makes it > succeed again: > > [1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) > [1671902.620706] EXT4-fs (loop1): group descriptors corrupted! > > Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> > --- > v2: New patch > --- > fs/ext4/ext4_common.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > index 621c61e5c7..31952f48b9 100644 > --- a/fs/ext4/ext4_common.c > +++ b/fs/ext4/ext4_common.c > @@ -432,6 +432,10 @@ uint16_t ext4fs_checksum_update(uint32_t i) > crc = ext2fs_crc16(crc, desc, offset); > offset += sizeof(desc->bg_checksum); /* skip checksum */ > assert(offset == sizeof(*desc)); > + if (offset < fs->gdsize) { > + crc = ext2fs_crc16(crc, (__u8 *)desc + offset, > + fs->gdsize - offset); > + } This would be feb0ab32a57e4e6c8b24f6fb68f0ce08efe4603c from the kernel? So shouldn't we have le16_to_cpu on fs->gdsize ? Or did I read over the 'git log -p' output in the kernel too quickly? Thanks!
On 09/26/2017 06:25 PM, Tom Rini wrote: > On Mon, Sep 25, 2017 at 10:06:31PM +0300, Tuomas Tynkkynen wrote: >> The current code doesn't compute the group descriptor checksum correctly >> for the filesystems that e2fsprogs 1.43.4 creates (they have >> 'Group descriptor size: 64' as reported by tune2fs). Extend the checksum >> calculation to be done as ext4_group_desc_csum() does in Linux. >> >> This fixes these errors in dmesg from running fs-test.sh and makes it >> succeed again: >> >> [1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) >> [1671902.620706] EXT4-fs (loop1): group descriptors corrupted! >> >> Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> >> --- >> v2: New patch >> --- >> fs/ext4/ext4_common.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c >> index 621c61e5c7..31952f48b9 100644 >> --- a/fs/ext4/ext4_common.c >> +++ b/fs/ext4/ext4_common.c >> @@ -432,6 +432,10 @@ uint16_t ext4fs_checksum_update(uint32_t i) >> crc = ext2fs_crc16(crc, desc, offset); >> offset += sizeof(desc->bg_checksum); /* skip checksum */ >> assert(offset == sizeof(*desc)); >> + if (offset < fs->gdsize) { >> + crc = ext2fs_crc16(crc, (__u8 *)desc + offset, >> + fs->gdsize - offset); >> + } > > This would be feb0ab32a57e4e6c8b24f6fb68f0ce08efe4603c from the kernel? No, it goes further back, to 2007 even(!): 717d50e4971b81b96c0199c91cdf0 > So shouldn't we have le16_to_cpu on fs->gdsize ? Or did I read over the > 'git log -p' output in the kernel too quickly? Thanks! > No, because fs->gdsize is already endian-swapped elsewhere: > 2345 fs->gdsize = le32_to_cpu(data->sblock.feature_incompat) & > 2346 EXT4_FEATURE_INCOMPAT_64BIT ? > 2347 le16_to_cpu(data->sblock.descriptor_size) : 32;
On Tue, Sep 26, 2017 at 07:04:06PM +0300, Tuomas Tynkkynen wrote: > On 09/26/2017 06:25 PM, Tom Rini wrote: > >On Mon, Sep 25, 2017 at 10:06:31PM +0300, Tuomas Tynkkynen wrote: > >>The current code doesn't compute the group descriptor checksum correctly > >>for the filesystems that e2fsprogs 1.43.4 creates (they have > >>'Group descriptor size: 64' as reported by tune2fs). Extend the checksum > >>calculation to be done as ext4_group_desc_csum() does in Linux. > >> > >>This fixes these errors in dmesg from running fs-test.sh and makes it > >>succeed again: > >> > >>[1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) > >>[1671902.620706] EXT4-fs (loop1): group descriptors corrupted! > >> > >>Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> > >>--- > >>v2: New patch > >>--- > >> fs/ext4/ext4_common.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >>diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c > >>index 621c61e5c7..31952f48b9 100644 > >>--- a/fs/ext4/ext4_common.c > >>+++ b/fs/ext4/ext4_common.c > >>@@ -432,6 +432,10 @@ uint16_t ext4fs_checksum_update(uint32_t i) > >> crc = ext2fs_crc16(crc, desc, offset); > >> offset += sizeof(desc->bg_checksum); /* skip checksum */ > >> assert(offset == sizeof(*desc)); > >>+ if (offset < fs->gdsize) { > >>+ crc = ext2fs_crc16(crc, (__u8 *)desc + offset, > >>+ fs->gdsize - offset); > >>+ } > > > >This would be feb0ab32a57e4e6c8b24f6fb68f0ce08efe4603c from the kernel? > > No, it goes further back, to 2007 even(!): 717d50e4971b81b96c0199c91cdf0 > > >So shouldn't we have le16_to_cpu on fs->gdsize ? Or did I read over the > >'git log -p' output in the kernel too quickly? Thanks! > > > > No, because fs->gdsize is already endian-swapped elsewhere: > > >2345 fs->gdsize = le32_to_cpu(data->sblock.feature_incompat) & > >2346 EXT4_FEATURE_INCOMPAT_64BIT ? > >2347 le16_to_cpu(data->sblock.descriptor_size) : 32; Ah, thanks for explaining and confirming. Reviewed-by; Tom Rini <trini@konsulko.com>
On Mon, Sep 25, 2017 at 10:06:31PM +0300, Tuomas Tynkkynen wrote: > The current code doesn't compute the group descriptor checksum correctly > for the filesystems that e2fsprogs 1.43.4 creates (they have > 'Group descriptor size: 64' as reported by tune2fs). Extend the checksum > calculation to be done as ext4_group_desc_csum() does in Linux. > > This fixes these errors in dmesg from running fs-test.sh and makes it > succeed again: > > [1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) > [1671902.620706] EXT4-fs (loop1): group descriptors corrupted! > > Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> Applied to u-boot/master, thanks!
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index 621c61e5c7..31952f48b9 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -432,6 +432,10 @@ uint16_t ext4fs_checksum_update(uint32_t i) crc = ext2fs_crc16(crc, desc, offset); offset += sizeof(desc->bg_checksum); /* skip checksum */ assert(offset == sizeof(*desc)); + if (offset < fs->gdsize) { + crc = ext2fs_crc16(crc, (__u8 *)desc + offset, + fs->gdsize - offset); + } } return crc;
The current code doesn't compute the group descriptor checksum correctly for the filesystems that e2fsprogs 1.43.4 creates (they have 'Group descriptor size: 64' as reported by tune2fs). Extend the checksum calculation to be done as ext4_group_desc_csum() does in Linux. This fixes these errors in dmesg from running fs-test.sh and makes it succeed again: [1671902.620699] EXT4-fs (loop1): ext4_check_descriptors: Checksum for group 0 failed (35782!=10965) [1671902.620706] EXT4-fs (loop1): group descriptors corrupted! Signed-off-by: Tuomas Tynkkynen <tuomas.tynkkynen@iki.fi> --- v2: New patch --- fs/ext4/ext4_common.c | 4 ++++ 1 file changed, 4 insertions(+)