| Message ID | 1226461390-5502-12-git-send-email-vaurora@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Theodore Ts'o |
| Headers | show |
On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: > Blocks per group and group desc count are both 32-bit; multiplied they > produce a 32-bit quantity which overflowed. > > @@ -92,8 +92,8 @@ errcode_t ext2fs_allocate_block_bitmap(ext2_filsys fs, > - real_end = (EXT2_BLOCKS_PER_GROUP(fs->super) > - * fs->group_desc_count)-1 + start; > + real_end = ((__u64) EXT2_BLOCKS_PER_GROUP(fs->super) > + * (__u64) fs->group_desc_count)-1 + start; Casting the first value to __u64 should be enough. This should really be part of patch 05/17 because that is where "real_end" is turned into a 64-bit value. There is a similar fix missing from ext2fs_allocate_inode_bitmap() (even though there isn't any support for 64-bit inode numbers as yet. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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
On Thu, Nov 13, 2008 at 01:04:02PM -0700, Andreas Dilger wrote: > On Nov 11, 2008 19:43 -0800, Valerie Aurora Henson wrote: > > Blocks per group and group desc count are both 32-bit; multiplied they > > produce a 32-bit quantity which overflowed. > > > > @@ -92,8 +92,8 @@ errcode_t ext2fs_allocate_block_bitmap(ext2_filsys fs, > > - real_end = (EXT2_BLOCKS_PER_GROUP(fs->super) > > - * fs->group_desc_count)-1 + start; > > + real_end = ((__u64) EXT2_BLOCKS_PER_GROUP(fs->super) > > + * (__u64) fs->group_desc_count)-1 + start; > > Casting the first value to __u64 should be enough. My approach with this kind of thing is to not make the reader try to remember the exact type conversion rules, but I'll take it out if it's just too much. > This should really be part of patch 05/17 because that is where "real_end" > is turned into a 64-bit value. Okay, I'll fold it back. -VAL -- 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
On Thu, Nov 13, 2008 at 01:04:02PM -0700, Andreas Dilger wrote: > > There is a similar fix missing from ext2fs_allocate_inode_bitmap() (even > though there isn't any support for 64-bit inode numbers as yet. That's a question I have: Should the 64-bit port have partial support for 64-bit inodes? Supporting 64-bit inodes is enough effort and risk (especially in terms of glue code to convert back to 32-bit versions) that I think it makes sense to wait until we know more about what they will look like and have concrete plans to support them. -VAL -- 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
On Nov 13, 2008 22:10 -0500, Valerie Aurora Henson wrote: > On Thu, Nov 13, 2008 at 01:04:02PM -0700, Andreas Dilger wrote: > > There is a similar fix missing from ext2fs_allocate_inode_bitmap() (even > > though there isn't any support for 64-bit inode numbers as yet. > > That's a question I have: Should the 64-bit port have partial support > for 64-bit inodes? Supporting 64-bit inodes is enough effort and risk > (especially in terms of glue code to convert back to 32-bit versions) > that I think it makes sense to wait until we know more about what they > will look like and have concrete plans to support them. I agree, in general, but this kind of wordsize overflow bug is easy to miss, and we may as well only have to figure out the bug once. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. -- 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 --git a/lib/ext2fs/bitmaps.c b/lib/ext2fs/bitmaps.c index f438a8b..91dc7a1 100644 --- a/lib/ext2fs/bitmaps.c +++ b/lib/ext2fs/bitmaps.c @@ -92,8 +92,8 @@ errcode_t ext2fs_allocate_block_bitmap(ext2_filsys fs, start = fs->super->s_first_data_block; end = ext2fs_blocks_count(fs->super)-1; - real_end = (EXT2_BLOCKS_PER_GROUP(fs->super) - * fs->group_desc_count)-1 + start; + real_end = ((__u64) EXT2_BLOCKS_PER_GROUP(fs->super) + * (__u64) fs->group_desc_count)-1 + start; if (fs->flags & EXT2_FLAG_NEW_BITMAPS) return (ext2fs_alloc_generic_bmap(fs,
Blocks per group and group desc count are both 32-bit; multiplied they produce a 32-bit quantity which overflowed. Signed-off-by: Valerie Aurora Henson <vaurora@redhat.com> --- lib/ext2fs/bitmaps.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)