| Message ID | 1429837961-2255-1-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Apr 24, 2015 at 09:12:40AM +0800, Qu Wenruo wrote: > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -173,6 +173,7 @@ struct btrfs_ordered_sum; > > /* csum types */ > #define BTRFS_CSUM_TYPE_CRC32 0 > +#define BTRFS_CSUM_LAST_TYPE 0 > > static int btrfs_csum_sizes[] = { 4, 0 }; I'd prefer to fix it by removing the 0 from btrfs_csum_sizes instead of introducing a define. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-------- Original Message -------- Subject: Re: [PATCH v2 1/2] btrfs: Fix superblock csum type check. From: David Sterba <dsterba@suse.cz> To: Qu Wenruo <quwenruo@cn.fujitsu.com> Date: 2015?04?24? 23:05 > On Fri, Apr 24, 2015 at 09:12:40AM +0800, Qu Wenruo wrote: >> --- a/fs/btrfs/ctree.h >> +++ b/fs/btrfs/ctree.h >> @@ -173,6 +173,7 @@ struct btrfs_ordered_sum; >> >> /* csum types */ >> #define BTRFS_CSUM_TYPE_CRC32 0 >> +#define BTRFS_CSUM_LAST_TYPE 0 >> >> static int btrfs_csum_sizes[] = { 4, 0 }; > > I'd prefer to fix it by removing the 0 from btrfs_csum_sizes instead of > introducing a define. > Removing the zero seems not help for this case, as some one can still craft a strange csum_type to access outside the array. So I introduce the new macro and use the new macro to compare with csum_type without acess the array. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 27, 2015 at 08:25:58AM +0800, Qu Wenruo wrote: > > > -------- Original Message -------- > Subject: Re: [PATCH v2 1/2] btrfs: Fix superblock csum type check. > From: David Sterba <dsterba@suse.cz> > To: Qu Wenruo <quwenruo@cn.fujitsu.com> > Date: 2015?04?24? 23:05 > > > On Fri, Apr 24, 2015 at 09:12:40AM +0800, Qu Wenruo wrote: > >> --- a/fs/btrfs/ctree.h > >> +++ b/fs/btrfs/ctree.h > >> @@ -173,6 +173,7 @@ struct btrfs_ordered_sum; > >> > >> /* csum types */ > >> #define BTRFS_CSUM_TYPE_CRC32 0 > >> +#define BTRFS_CSUM_LAST_TYPE 0 > >> > >> static int btrfs_csum_sizes[] = { 4, 0 }; > > > > I'd prefer to fix it by removing the 0 from btrfs_csum_sizes instead of > > introducing a define. > > > > Removing the zero seems not help for this case, as some one can still > craft a strange csum_type to access outside the array. The ARRAY_SIZE will be 1, so if a crafted csum will be anything than 0, then the check will catch it, no? > So I introduce the new macro and use the new macro to compare with > csum_type without acess the array. The macro serves the same purpose as the ARRAY_SIZE macro and is always in sync with the btrfs_csum_size. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-------- Original Message -------- Subject: Re: [PATCH v2 1/2] btrfs: Fix superblock csum type check. From: David Sterba <dsterba@suse.cz> To: Qu Wenruo <quwenruo@cn.fujitsu.com> Date: 2015?04?27? 18:59 > On Mon, Apr 27, 2015 at 08:25:58AM +0800, Qu Wenruo wrote: >> >> >> -------- Original Message -------- >> Subject: Re: [PATCH v2 1/2] btrfs: Fix superblock csum type check. >> From: David Sterba <dsterba@suse.cz> >> To: Qu Wenruo <quwenruo@cn.fujitsu.com> >> Date: 2015?04?24? 23:05 >> >>> On Fri, Apr 24, 2015 at 09:12:40AM +0800, Qu Wenruo wrote: >>>> --- a/fs/btrfs/ctree.h >>>> +++ b/fs/btrfs/ctree.h >>>> @@ -173,6 +173,7 @@ struct btrfs_ordered_sum; >>>> >>>> /* csum types */ >>>> #define BTRFS_CSUM_TYPE_CRC32 0 >>>> +#define BTRFS_CSUM_LAST_TYPE 0 >>>> >>>> static int btrfs_csum_sizes[] = { 4, 0 }; >>> >>> I'd prefer to fix it by removing the 0 from btrfs_csum_sizes instead of >>> introducing a define. >>> >> >> Removing the zero seems not help for this case, as some one can still >> craft a strange csum_type to access outside the array. > > The ARRAY_SIZE will be 1, so if a crafted csum will be anything than 0, > then the check will catch it, no? Oh, I forgot there is ARRAY_SIZE check. Now deleting the 0 in array is definitely the cleanest fix. Thanks, Qu > >> So I introduce the new macro and use the new macro to compare with >> csum_type without acess the array. > > The macro serves the same purpose as the ARRAY_SIZE macro and is always > in sync with the btrfs_csum_size. > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index f9c89ca..d6f3aa0 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -173,6 +173,7 @@ struct btrfs_ordered_sum; /* csum types */ #define BTRFS_CSUM_TYPE_CRC32 0 +#define BTRFS_CSUM_LAST_TYPE 0 static int btrfs_csum_sizes[] = { 4, 0 }; diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 639f266..e33a01b 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -426,9 +426,9 @@ static int btrfs_check_super_csum(char *raw_disk_sb) } } - if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) { + if (csum_type > BTRFS_CSUM_LAST_TYPE) { printk(KERN_ERR "BTRFS: unsupported checksum algorithm %u\n", - csum_type); + csum_type); ret = 1; }
Old csum type check is wrong and can't catch csum_type 1(not supported). Fix it to avoid hostile 0 division. Reported-by: Lukas Lueg <lukas.lueg@gmail.com> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- Changelog: v2: Fix existing codes other than adding new one. --- fs/btrfs/ctree.h | 1 + fs/btrfs/disk-io.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)