diff mbox

[11/37] libext2fs: Create the inode bitmap checksum

Message ID 20110901003622.1176.1504.stgit@elm3c44.beaverton.ibm.com
State Superseded, archived
Headers show

Commit Message

Darrick J. Wong Sept. 1, 2011, 12:36 a.m. UTC
Provide a field in the block group descriptor to store inode bitmap checksum,
and some helper functions to calculate and verify it.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---
 lib/ext2fs/blknum.c     |   11 +++++++++++
 lib/ext2fs/closefs.c    |   29 +++++++++++++++++------------
 lib/ext2fs/csum.c       |   45 +++++++++++++++++++++++++++++++++++++++++++++
 lib/ext2fs/ext2_fs.h    |    3 ++-
 lib/ext2fs/ext2fs.h     |    7 +++++++
 lib/ext2fs/rw_bitmaps.c |   15 +++++++++++++++
 lib/ext2fs/swapfs.c     |    1 +
 7 files changed, 98 insertions(+), 13 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Theodore Ts'o Sept. 14, 2011, 5:02 p.m. UTC | #1
On Wed, Aug 31, 2011 at 05:36:22PM -0700, Darrick J. Wong wrote:
> diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
> index 1f08673..367bfdf 100644
> --- a/lib/ext2fs/ext2_fs.h
> +++ b/lib/ext2fs/ext2_fs.h
> @@ -169,7 +169,8 @@ struct ext4_group_desc
>  	__u16	bg_free_inodes_count_hi;/* Free inodes count MSB */
>  	__u16	bg_used_dirs_count_hi;	/* Directories count MSB */
>  	__u16	bg_itable_unused_hi;	/* Unused inodes count MSB */
> -	__u32	bg_reserved2[3];
> +	__u32	bg_inode_bitmap_csum;	/* crc32c(uuid+group+ibitmap) */
> +	__u32	bg_reserved2[2];
>  };


One of the reasons why I like to coalesce the patches to the data
structures into their own separate commit, is it's hard when I'm
reviewing individual patches in a mail reader what's going on from a
big picture spective.  (Heck, even just *finding* the patches that
modify the on-disk format is hard....)

But as near as I can tell, your patch series only uses one of the
32-bit fields in bg_reserved.  Is there a good reason why
bg_inode_bitmap_csum can't also used one of the two fields in
bg_reserved?  That way we get two 32-bit checksums for both struct
ext2_group_desc and struct ext4_group_desc.  Is there a third 32-bit
per-block group checksum I'm forgetting about? 

	  		     		       - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Darrick J. Wong Sept. 14, 2011, 7:31 p.m. UTC | #2
On Wed, Sep 14, 2011 at 01:02:59PM -0400, Ted Ts'o wrote:
> On Wed, Aug 31, 2011 at 05:36:22PM -0700, Darrick J. Wong wrote:
> > diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
> > index 1f08673..367bfdf 100644
> > --- a/lib/ext2fs/ext2_fs.h
> > +++ b/lib/ext2fs/ext2_fs.h
> > @@ -169,7 +169,8 @@ struct ext4_group_desc
> >  	__u16	bg_free_inodes_count_hi;/* Free inodes count MSB */
> >  	__u16	bg_used_dirs_count_hi;	/* Directories count MSB */
> >  	__u16	bg_itable_unused_hi;	/* Unused inodes count MSB */
> > -	__u32	bg_reserved2[3];
> > +	__u32	bg_inode_bitmap_csum;	/* crc32c(uuid+group+ibitmap) */
> > +	__u32	bg_reserved2[2];
> >  };
> 
> 
> One of the reasons why I like to coalesce the patches to the data
> structures into their own separate commit, is it's hard when I'm
> reviewing individual patches in a mail reader what's going on from a
> big picture spective.  (Heck, even just *finding* the patches that
> modify the on-disk format is hard....)
> 
> But as near as I can tell, your patch series only uses one of the
> 32-bit fields in bg_reserved.  Is there a good reason why
> bg_inode_bitmap_csum can't also used one of the two fields in
> bg_reserved?  That way we get two 32-bit checksums for both struct
> ext2_group_desc and struct ext4_group_desc.  Is there a third 32-bit
> per-block group checksum I'm forgetting about? 

No, but I was under the impression that Amir was using one of bg_reserved for
snapshots.  If that's not true, I'll move both bitmap checksums to bg_reserved.

--D
> 
> 	  		     		       - Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Dilger Sept. 14, 2011, 7:59 p.m. UTC | #3
On 2011-09-14, at 11:02 AM, Ted Ts'o wrote:
> On Wed, Aug 31, 2011 at 05:36:22PM -0700, Darrick J. Wong wrote:
>> diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
>> index 1f08673..367bfdf 100644
>> --- a/lib/ext2fs/ext2_fs.h
>> +++ b/lib/ext2fs/ext2_fs.h
>> @@ -169,7 +169,8 @@ struct ext4_group_desc
>> 	__u16	bg_free_inodes_count_hi;/* Free inodes count MSB */
>> 	__u16	bg_used_dirs_count_hi;	/* Directories count MSB */
>> 	__u16	bg_itable_unused_hi;	/* Unused inodes count MSB */
>> -	__u32	bg_reserved2[3];
>> +	__u32	bg_inode_bitmap_csum;	/* crc32c(uuid+group+ibitmap) */
>> +	__u32	bg_reserved2[2];
>> };
> 
> One of the reasons why I like to coalesce the patches to the data
> structures into their own separate commit, is it's hard when I'm
> reviewing individual patches in a mail reader what's going on from a
> big picture spective.  (Heck, even just *finding* the patches that
> modify the on-disk format is hard....)
> 
> But as near as I can tell, your patch series only uses one of the
> 32-bit fields in bg_reserved.  Is there a good reason why
> bg_inode_bitmap_csum can't also used one of the two fields in
> bg_reserved?  That way we get two 32-bit checksums for both struct
> ext2_group_desc and struct ext4_group_desc.  Is there a third 32-bit
> per-block group checksum I'm forgetting about?

There is the field that you told Amir he could use for the exception
bitmap for snapshots, which is using one of the two reserved fields in
ext2_group_desc, and also one of the 3 reserved fields in ext4_group_desc
for 64-bit block numbers.  That leaves one __u32 in ext2_group_desc, and
two __u32 in ext4_group_desc for checksums.

My proposal was as follows.  It adds the split checksums for block and
inode bitmaps, and renames bg_checksum to bg_group_desc_csum to avoid
confusion with these new checksums.  Truncate after bg_group_desc_csum
for smaller ext2_group_desc version.

struct ext4_group_desc
{
        __le32  bg_block_bitmap_lo;     /* Blocks bitmap block */
        __le32  bg_inode_bitmap_lo;     /* Inodes bitmap block */
        __le32  bg_inode_table_lo;      /* Inodes table block */
        __le16  bg_free_blocks_count_lo;/* Free blocks count */
        __le16  bg_free_inodes_count_lo;/* Free inodes count */
        __le16  bg_used_dirs_count_lo;  /* Directories count */
        __le16  bg_flags;               /* EXT4_BG_flags (INODE_UNINIT, etc) */
        __le32  bg_exclude_bitmap_lo;   /* Snapshot exclude bitmap block LSB */
        __le16  bg_block_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */
        __le16  bg_inode_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */
        __le16  bg_itable_unused_lo;    /* Unused inodes count */
        __le16  bg_group_desc_csum;     /* crc16(sb_uuid+group+desc) */
        __le32  bg_block_bitmap_hi;     /* Blocks bitmap block MSB */
        __le32  bg_inode_bitmap_hi;     /* Inodes bitmap block MSB */
        __le32  bg_inode_table_hi;      /* Inodes table block MSB */
        __le16  bg_free_blocks_count_hi;/* Free blocks count MSB */
        __le16  bg_free_inodes_count_hi;/* Free inodes count MSB */
        __le16  bg_used_dirs_count_hi;  /* Directories count MSB */
        __le16  bg_itable_unused_hi;    /* Unused inodes count MSB */
	__le32	bg_exclude_bitmap_hi;	/* Exclude bitmap block MSB */
        __le16  bg_block_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */
        __le16  bg_inode_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */
        __le32  bg_reserved2;
};

Whether crc32c & 0xffff is as strong as crc16 is open for debate, but it
isn't completely worthless (it provides some protection, and crc32c is
much faster at least for larger block sizes), and is only for filesystems
upgraded in-place from ext3 so it isn't critical in the long run.  I'd
rather have less complex and faster code than having to conditionally
compute crc16 vs crc32c depending on the ext4_group_desc size.

There is also the question if we want to extend bg_group_desc_csum and/or
the bg_flags values to be 32-bit fields?  If we do, then that uses up all
of the space in the 64-byte group descriptor and we would need to bump it
to 128 bytes to add any new fields.  We can deal with that when we run
out of flags.  I don't think there is a need to have a 32-bit checksum
for such a small structure, and potentially it makes sense to use the low
bits of crc32c instead of crc16 just to stick with a single algorithm for
the filesystem.

Cheers, Andreas--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Dilger Sept. 14, 2011, 8 p.m. UTC | #4
On 2011-09-14, at 1:31 PM, Darrick J. Wong wrote:
> On Wed, Sep 14, 2011 at 01:02:59PM -0400, Ted Ts'o wrote:
>> On Wed, Aug 31, 2011 at 05:36:22PM -0700, Darrick J. Wong wrote:
>>> diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
>>> index 1f08673..367bfdf 100644
>>> --- a/lib/ext2fs/ext2_fs.h
>>> +++ b/lib/ext2fs/ext2_fs.h
>>> @@ -169,7 +169,8 @@ struct ext4_group_desc
>>> 	__u16	bg_free_inodes_count_hi;/* Free inodes count MSB */
>>> 	__u16	bg_used_dirs_count_hi;	/* Directories count MSB */
>>> 	__u16	bg_itable_unused_hi;	/* Unused inodes count MSB */
>>> -	__u32	bg_reserved2[3];
>>> +	__u32	bg_inode_bitmap_csum;	/* crc32c(uuid+group+ibitmap) */
>>> +	__u32	bg_reserved2[2];
>>> };
>> 
>> 
>> One of the reasons why I like to coalesce the patches to the data
>> structures into their own separate commit, is it's hard when I'm
>> reviewing individual patches in a mail reader what's going on from a
>> big picture spective.  (Heck, even just *finding* the patches that
>> modify the on-disk format is hard....)
>> 
>> But as near as I can tell, your patch series only uses one of the
>> 32-bit fields in bg_reserved.  Is there a good reason why
>> bg_inode_bitmap_csum can't also used one of the two fields in
>> bg_reserved?  That way we get two 32-bit checksums for both struct
>> ext2_group_desc and struct ext4_group_desc.  Is there a third 32-bit
>> per-block group checksum I'm forgetting about? 
> 
> No, but I was under the impression that Amir was using one of bg_reserved for
> snapshots.  If that's not true, I'll move both bitmap checksums to bg_reserved.

He was, AND the first bg_reserved2 field as well, which the above use
would conflict with.  See my other patch.

Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o Sept. 14, 2011, 10:14 p.m. UTC | #5
On Wed, Sep 14, 2011 at 01:59:06PM -0600, Andreas Dilger wrote:
> 
> There is the field that you told Amir he could use for the exception
> bitmap for snapshots, which is using one of the two reserved fields in
> ext2_group_desc, and also one of the 3 reserved fields in ext4_group_desc
> for 64-bit block numbers.  That leaves one __u32 in ext2_group_desc, and
> two __u32 in ext4_group_desc for checksums.

Right, that's what I was forgetting.  Thanks for reminding me!

> My proposal was as follows.  It adds the split checksums for block and
> inode bitmaps, and renames bg_checksum to bg_group_desc_csum to avoid
> confusion with these new checksums.  Truncate after bg_group_desc_csum
> for smaller ext2_group_desc version.
> 
> struct ext4_group_desc
> {
>         __le32  bg_block_bitmap_lo;     /* Blocks bitmap block */
>         __le32  bg_inode_bitmap_lo;     /* Inodes bitmap block */
>         __le32  bg_inode_table_lo;      /* Inodes table block */
>         __le16  bg_free_blocks_count_lo;/* Free blocks count */
>         __le16  bg_free_inodes_count_lo;/* Free inodes count */
>         __le16  bg_used_dirs_count_lo;  /* Directories count */
>         __le16  bg_flags;               /* EXT4_BG_flags (INODE_UNINIT, etc) */
>         __le32  bg_exclude_bitmap_lo;   /* Snapshot exclude bitmap block LSB */
>         __le16  bg_block_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */
>         __le16  bg_inode_bitmap_csum_lo;/* crc32c(s_uuid+grp_num+bitmap) LSB */
>         __le16  bg_itable_unused_lo;    /* Unused inodes count */
>         __le16  bg_group_desc_csum;     /* crc16(sb_uuid+group+desc) */
>         __le32  bg_block_bitmap_hi;     /* Blocks bitmap block MSB */
>         __le32  bg_inode_bitmap_hi;     /* Inodes bitmap block MSB */
>         __le32  bg_inode_table_hi;      /* Inodes table block MSB */
>         __le16  bg_free_blocks_count_hi;/* Free blocks count MSB */
>         __le16  bg_free_inodes_count_hi;/* Free inodes count MSB */
>         __le16  bg_used_dirs_count_hi;  /* Directories count MSB */
>         __le16  bg_itable_unused_hi;    /* Unused inodes count MSB */
> 	__le32	bg_exclude_bitmap_hi;	/* Exclude bitmap block MSB */
>         __le16  bg_block_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */
>         __le16  bg_inode_bitmap_csum_hi;/* crc32c(s_uuid+grp_num+bitmap) MSB */
>         __le32  bg_reserved2;
> };

That looks reasonable to me.

> Whether crc32c & 0xffff is as strong as crc16 is open for debate, but it
> isn't completely worthless (it provides some protection, and crc32c is
> much faster at least for larger block sizes), and is only for filesystems
> upgraded in-place from ext3 so it isn't critical in the long run.  I'd
> rather have less complex and faster code than having to conditionally
> compute crc16 vs crc32c depending on the ext4_group_desc size.

That seems reasonable for me.  Note that most ext4 file systems are
still being created using the 32 byte bg descriptor, mainly for
compatibility for older 1.41.x e2fsprogs.  But yes, long term that's
fair.

> There is also the question if we want to extend bg_group_desc_csum and/or
> the bg_flags values to be 32-bit fields?  If we do, then that uses up all
> of the space in the 64-byte group descriptor and we would need to bump it
> to 128 bytes to add any new fields.  We can deal with that when we run
> out of flags.

I don't think either is likely; we've used very few flags in bg_flags
so far, and the bg_group_desc_csum is only covering 64 bytes.  A
16-bit checksum should be more than good enough for that.

The thing which did occur to me was whether it would make sence to
change the use of crc16(x) to crc32(x) & 0xffff if the METADATA_CSUM
feature is enabled for the existing bg_checksum field.  But the speed
benefits for checksuming such a small number of bytes is minimal, and
probably not worth the extra code complexity.

Regards,

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index 7e7fcd8..47d3fda 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -210,6 +210,17 @@  void ext2fs_block_bitmap_loc_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
 }
 
 /*
+ * Return the inode bitmap checksum of a group
+ */
+blk64_t ext2fs_inode_bitmap_checksum(ext2_filsys fs, dgrp_t group)
+{
+	struct ext4_group_desc *gdp;
+
+	gdp = ext4fs_group_desc(fs, fs->group_desc, group);
+	return gdp->bg_inode_bitmap_csum;
+}
+
+/*
  * Return the inode bitmap block of a group
  */
 blk64_t ext2fs_inode_bitmap_loc(ext2_filsys fs, dgrp_t group)
diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 952f496..73dc136 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -289,6 +289,23 @@  errcode_t ext2fs_flush(ext2_filsys fs)
 
 	fs->super->s_wtime = fs->now ? fs->now : time(NULL);
 	fs->super->s_block_group_nr = 0;
+
+	/*
+	 * If the write_bitmaps() function is present, call it to
+	 * flush the bitmaps.  This is done this way so that a simple
+	 * program that doesn't mess with the bitmaps doesn't need to
+	 * drag in the bitmaps.c code.
+	 *
+	 * Bitmap checksums live in the group descriptor, so the
+	 * bitmaps need to be written before the descriptors.
+	 */
+	if (fs->write_bitmaps) {
+		retval = fs->write_bitmaps(fs);
+		if (retval)
+			goto errout;
+	}
+
+	/* Prepare the group descriptors for writing */
 #ifdef WORDS_BIGENDIAN
 	retval = EXT2_ET_NO_MEMORY;
 	retval = ext2fs_get_mem(SUPERBLOCK_SIZE, &super_shadow);
@@ -379,18 +396,6 @@  errcode_t ext2fs_flush(ext2_filsys fs)
 
 	ext2fs_numeric_progress_close(fs, &progress, NULL);
 
-	/*
-	 * If the write_bitmaps() function is present, call it to
-	 * flush the bitmaps.  This is done this way so that a simple
-	 * program that doesn't mess with the bitmaps doesn't need to
-	 * drag in the bitmaps.c code.
-	 */
-	if (fs->write_bitmaps) {
-		retval = fs->write_bitmaps(fs);
-		if (retval)
-			goto errout;
-	}
-
 write_primary_superblock_only:
 	/*
 	 * Write out master superblock.  This has to be done
diff --git a/lib/ext2fs/csum.c b/lib/ext2fs/csum.c
index 57adc4c..56b75da 100644
--- a/lib/ext2fs/csum.c
+++ b/lib/ext2fs/csum.c
@@ -29,6 +29,51 @@ 
 #define STATIC static
 #endif
 
+__u32 ext2fs_bitmap_csum(ext2_filsys fs, dgrp_t group, char *bitmap, int size)
+{
+	__u32 crc = 0;
+
+	if (fs->super->s_desc_size < EXT2_MIN_DESC_SIZE_64BIT)
+		return 0;
+
+	if (!EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
+					EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
+		return 0;
+
+	group = ext2fs_cpu_to_le32(group);
+	crc = crc32c_le(~0, fs->super->s_uuid, sizeof(fs->super->s_uuid));
+	crc = crc32c_le(crc, (char *)&group, sizeof(group));
+	crc = crc32c_le(crc, (char *)bitmap, size);
+
+	return crc;
+}
+
+int ext2fs_bitmap_csum_verify(ext2_filsys fs, dgrp_t group, __u32 provided,
+			      char *bitmap, int size)
+{
+	if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT &&
+	    EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
+				       EXT4_FEATURE_RO_COMPAT_METADATA_CSUM) &&
+	    (provided != ext2fs_bitmap_csum(fs, group, bitmap, size)))
+		return 0;
+	return 1;
+}
+
+void ext2fs_inode_bitmap_csum_set(ext2_filsys fs, dgrp_t group, char *bitmap,
+				  int size)
+{
+	struct ext4_group_desc *gdp = (struct ext4_group_desc *)
+			ext2fs_group_desc(fs, fs->group_desc, group);
+
+	if (fs->super->s_desc_size < EXT2_MIN_DESC_SIZE_64BIT)
+		return;
+	if (!EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
+					EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
+		return;
+
+	gdp->bg_inode_bitmap_csum = ext2fs_bitmap_csum(fs, group, bitmap, size);
+}
+
 __u32 ext2fs_inode_csum(ext2_filsys fs, ext2_ino_t inum,
 			struct ext2_inode_large *inode)
 {
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index 1f08673..367bfdf 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -169,7 +169,8 @@  struct ext4_group_desc
 	__u16	bg_free_inodes_count_hi;/* Free inodes count MSB */
 	__u16	bg_used_dirs_count_hi;	/* Directories count MSB */
 	__u16	bg_itable_unused_hi;	/* Unused inodes count MSB */
-	__u32	bg_reserved2[3];
+	__u32	bg_inode_bitmap_csum;	/* crc32c(uuid+group+ibitmap) */
+	__u32	bg_reserved2[2];
 };
 
 #define EXT2_BG_INODE_UNINIT	0x0001 /* Inode table/bitmap not initialized */
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index db8b28b..0899e34 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -785,6 +785,7 @@  extern struct ext2_group_desc *ext2fs_group_desc(ext2_filsys fs,
 extern blk64_t ext2fs_block_bitmap_loc(ext2_filsys fs, dgrp_t group);
 extern void ext2fs_block_bitmap_loc_set(ext2_filsys fs, dgrp_t group,
 					blk64_t blk);
+extern blk64_t ext2fs_inode_bitmap_csum(ext2_filsys fs, dgrp_t group);
 extern blk64_t ext2fs_inode_bitmap_loc(ext2_filsys fs, dgrp_t group);
 extern void ext2fs_inode_bitmap_loc_set(ext2_filsys fs, dgrp_t group,
 					blk64_t blk);
@@ -892,6 +893,12 @@  extern __u32 crc32c_be(__u32 crc, unsigned char const *p, size_t len);
 extern __u32 crc32c_le(__u32 crc, unsigned char const *p, size_t len);
 
 /* csum.c */
+extern __u32 ext2fs_bitmap_csum(ext2_filsys fs, dgrp_t group, char *bitmap,
+				int size);
+extern int ext2fs_bitmap_csum_verify(ext2_filsys fs, dgrp_t group,
+				     __u32 provided, char *bitmap, int size);
+extern void ext2fs_inode_bitmap_csum_set(ext2_filsys fs, dgrp_t group,
+					 char *bitmap, int size);
 extern __u32 ext2fs_inode_csum(ext2_filsys fs, ext2_ino_t inum,
 			      struct ext2_inode_large *inode);
 extern void ext2fs_inode_csum_set(ext2_filsys fs, ext2_ino_t inum,
diff --git a/lib/ext2fs/rw_bitmaps.c b/lib/ext2fs/rw_bitmaps.c
index f8c8a9f..57aba59 100644
--- a/lib/ext2fs/rw_bitmaps.c
+++ b/lib/ext2fs/rw_bitmaps.c
@@ -114,6 +114,9 @@  static errcode_t write_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 		if (retval)
 			return retval;
 
+		ext2fs_inode_bitmap_csum_set(fs, i, inode_buf, inode_nbytes);
+		ext2fs_group_desc_csum_set(fs, i);
+
 		blk = ext2fs_inode_bitmap_loc(fs, i);
 		if (blk) {
 			retval = io_channel_write_blk64(fs->io, blk, 1,
@@ -152,6 +155,7 @@  static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 	blk64_t   blk_cnt;
 	ext2_ino_t ino_itr = 1;
 	ext2_ino_t ino_cnt;
+	struct ext4_group_desc *gdp;
 
 	EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
 
@@ -277,6 +281,17 @@  static errcode_t read_bitmaps(ext2_filsys fs, int do_inode, int do_block)
 					retval = EXT2_ET_INODE_BITMAP_READ;
 					goto cleanup;
 				}
+
+				/* verify inode bitmap checksum */
+				gdp = (struct ext4_group_desc *)
+					ext2fs_group_desc(fs, fs->group_desc,
+							  i);
+				if (!(fs->flags &
+				      EXT2_FLAG_IGNORE_CSUM_ERRORS) &&
+				    !ext2fs_bitmap_csum_verify(fs, i,
+					gdp->bg_inode_bitmap_csum,
+					inode_bitmap, inode_nbytes))
+					return EXT2_ET_INODE_BITMAP_READ;
 			} else
 				memset(inode_bitmap, 0, inode_nbytes);
 			cnt = inode_nbytes << 3;
diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
index df604ba..747e130 100644
--- a/lib/ext2fs/swapfs.c
+++ b/lib/ext2fs/swapfs.c
@@ -125,6 +125,7 @@  void ext2fs_swap_group_desc2(ext2_filsys fs, struct ext2_group_desc *gdp)
 	gdp4->bg_used_dirs_count_hi =
 		ext2fs_swab16(gdp4->bg_used_dirs_count_hi);
 	gdp4->bg_itable_unused_hi = ext2fs_swab16(gdp4->bg_itable_unused_hi);
+	gdp4->bg_inode_bitmap_csum = ext2fs_swab32(gdp4->bg_inode_bitmap_csum);
 }
 
 void ext2fs_swap_group_desc(struct ext2_group_desc *gdp)