| Message ID | 1461718411-809-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > The struct 'map_lookup' uses type int for @stripe_len, while > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > @stripe_len being undefined value and it can lead to 'divide error' in > __btrfs_map_block(). > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > BTRFS_STRIPE_LEN. > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> I smell some fuzzing :) do you have the image available? I'll add it to the rest in btrfsprogs. > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > --- > fs/btrfs/volumes.c | 2 +- > fs/btrfs/volumes.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index e2b54d5..b5cb859 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > "invalid chunk length %llu", length); > return -EIO; > } > - if (!is_power_of_2(stripe_len)) { > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { Unfortunatelly this will break current state, as mkfs does not set the stripe length to 64k but to 4k. But the value is otherwise ignored in kernel. -- 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 Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: > On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > > The struct 'map_lookup' uses type int for @stripe_len, while > > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > > @stripe_len being undefined value and it can lead to 'divide error' in > > __btrfs_map_block(). > > > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > > BTRFS_STRIPE_LEN. > > > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > > I smell some fuzzing :) do you have the image available? I'll add it to > the rest in btrfsprogs. Sure, it's on the way, I'll send it along with a patch for btrfsck (we have to add the same validation check for superblock and chunk in btrfsck.) > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > --- > > fs/btrfs/volumes.c | 2 +- > > fs/btrfs/volumes.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > index e2b54d5..b5cb859 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > > "invalid chunk length %llu", length); > > return -EIO; > > } > > - if (!is_power_of_2(stripe_len)) { > > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > Unfortunatelly this will break current state, as mkfs does not set the > stripe length to 64k but to 4k. But the value is otherwise ignored in > kernel. This is chunk's stripe_len, not superblock's stripe_len: make_btrfs() { ... btrfs_set_super_stripesize(&super, cfg->stripesize); --> 4096 ... btrfs_set_chunk_stripe_len(buf, chunk, 64 * 1024); } Thanks, -liubo -- 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 Wed, Apr 27, 2016 at 10:23:35AM -0700, Liu Bo wrote: > On Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: > > On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > > > The struct 'map_lookup' uses type int for @stripe_len, while > > > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > > > @stripe_len being undefined value and it can lead to 'divide error' in > > > __btrfs_map_block(). > > > > > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > > > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > > > BTRFS_STRIPE_LEN. > > > > > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > > > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > > > > I smell some fuzzing :) do you have the image available? I'll add it to > > the rest in btrfsprogs. > > Sure, it's on the way, I'll send it along with a patch for btrfsck (we > have to add the same validation check for superblock and chunk in > btrfsck.) Great! > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > > --- > > > fs/btrfs/volumes.c | 2 +- > > > fs/btrfs/volumes.h | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > > index e2b54d5..b5cb859 100644 > > > --- a/fs/btrfs/volumes.c > > > +++ b/fs/btrfs/volumes.c > > > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > > > "invalid chunk length %llu", length); > > > return -EIO; > > > } > > > - if (!is_power_of_2(stripe_len)) { > > > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > > > Unfortunatelly this will break current state, as mkfs does not set the > > stripe length to 64k but to 4k. But the value is otherwise ignored in > > kernel. > > This is chunk's stripe_len, not superblock's stripe_len: > > make_btrfs() { > ... > btrfs_set_super_stripesize(&super, cfg->stripesize); --> 4096 > ... > btrfs_set_chunk_stripe_len(buf, chunk, 64 * 1024); > } Oh right, and the hardcoded stripe chunk size would need to be fixed in a lot more places so it's fine. Consider this Reviewed-by: David Sterba <dsterba@suse.com> and on the way to for-next. -- 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 Wed, Apr 27, 2016 at 07:33:18PM +0200, David Sterba wrote: > On Wed, Apr 27, 2016 at 10:23:35AM -0700, Liu Bo wrote: > > On Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: > > > On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > > > > The struct 'map_lookup' uses type int for @stripe_len, while > > > > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > > > > @stripe_len being undefined value and it can lead to 'divide error' in > > > > __btrfs_map_block(). > > > > > > > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > > > > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > > > > BTRFS_STRIPE_LEN. > > > > > > > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > > > > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > > > > > > I smell some fuzzing :) do you have the image available? I'll add it to > > > the rest in btrfsprogs. > > > > Sure, it's on the way, I'll send it along with a patch for btrfsck (we > > have to add the same validation check for superblock and chunk in > > btrfsck.) > > Great! > > > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > > > --- > > > > fs/btrfs/volumes.c | 2 +- > > > > fs/btrfs/volumes.h | 2 +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > > > index e2b54d5..b5cb859 100644 > > > > --- a/fs/btrfs/volumes.c > > > > +++ b/fs/btrfs/volumes.c > > > > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > > > > "invalid chunk length %llu", length); > > > > return -EIO; > > > > } > > > > - if (!is_power_of_2(stripe_len)) { > > > > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { We don't need the first 'is_power_of_2' check. And I think we may need to have another helper, such as btrfs_check_chunk_valid(), to cover all these (both current and future) validation checks. What do you think? Thanks, -liubo > > > > > > Unfortunatelly this will break current state, as mkfs does not set the > > > stripe length to 64k but to 4k. But the value is otherwise ignored in > > > kernel. > > > > This is chunk's stripe_len, not superblock's stripe_len: > > > > make_btrfs() { > > ... > > btrfs_set_super_stripesize(&super, cfg->stripesize); --> 4096 > > ... > > btrfs_set_chunk_stripe_len(buf, chunk, 64 * 1024); > > } > > Oh right, and the hardcoded stripe chunk size would need to be fixed in > a lot more places so it's fine. Consider this > > Reviewed-by: David Sterba <dsterba@suse.com> > > and on the way to for-next. -- 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
Liu Bo wrote on 2016/04/28 10:48 -0700: > On Wed, Apr 27, 2016 at 07:33:18PM +0200, David Sterba wrote: >> On Wed, Apr 27, 2016 at 10:23:35AM -0700, Liu Bo wrote: >>> On Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: >>>> On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: >>>>> The struct 'map_lookup' uses type int for @stripe_len, while >>>>> btrfs_chunk_stripe_len() can return a u64 value, and it may end up with >>>>> @stripe_len being undefined value and it can lead to 'divide error' in >>>>> __btrfs_map_block(). >>>>> >>>>> This changes 'map_lookup' to use type u64 for stripe_len, also right now >>>>> we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for >>>>> BTRFS_STRIPE_LEN. >>>>> >>>>> Reported-by: Vegard Nossum <vegard.nossum@oracle.com> >>>>> Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> >>>> >>>> I smell some fuzzing :) do you have the image available? I'll add it to >>>> the rest in btrfsprogs. >>> >>> Sure, it's on the way, I'll send it along with a patch for btrfsck (we >>> have to add the same validation check for superblock and chunk in >>> btrfsck.) >> >> Great! >> >>>>> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> >>>>> --- >>>>> fs/btrfs/volumes.c | 2 +- >>>>> fs/btrfs/volumes.h | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>>>> index e2b54d5..b5cb859 100644 >>>>> --- a/fs/btrfs/volumes.c >>>>> +++ b/fs/btrfs/volumes.c >>>>> @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, >>>>> "invalid chunk length %llu", length); >>>>> return -EIO; >>>>> } >>>>> - if (!is_power_of_2(stripe_len)) { >>>>> + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > We don't need the first 'is_power_of_2' check. > > And I think we may need to have another helper, such as btrfs_check_chunk_valid(), > to cover all these (both current and future) validation checks. What do you think? > > Thanks, > > -liubo > +1 for a btrfs_check_chunk_valid(). And I'm OK to remove is_power_of_2 check, as it's only used for future stripe_len. But we only support fix STRIPE_LEN, it's OK to remove it. BTW, did it crash btrfs-progs? Thanks, Qu >>>> >>>> Unfortunatelly this will break current state, as mkfs does not set the >>>> stripe length to 64k but to 4k. But the value is otherwise ignored in >>>> kernel. >>> >>> This is chunk's stripe_len, not superblock's stripe_len: >>> >>> make_btrfs() { >>> ... >>> btrfs_set_super_stripesize(&super, cfg->stripesize); --> 4096 >>> ... >>> btrfs_set_chunk_stripe_len(buf, chunk, 64 * 1024); >>> } >> >> Oh right, and the hardcoded stripe chunk size would need to be fixed in >> a lot more places so it's fine. Consider this >> >> Reviewed-by: David Sterba <dsterba@suse.com> >> >> and on the way to for-next. > -- > 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
On Thu, Apr 28, 2016 at 10:48:36AM -0700, Liu Bo wrote: > On Wed, Apr 27, 2016 at 07:33:18PM +0200, David Sterba wrote: > > On Wed, Apr 27, 2016 at 10:23:35AM -0700, Liu Bo wrote: > > > On Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: > > > > On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > > > > > The struct 'map_lookup' uses type int for @stripe_len, while > > > > > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > > > > > @stripe_len being undefined value and it can lead to 'divide error' in > > > > > __btrfs_map_block(). > > > > > > > > > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > > > > > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > > > > > BTRFS_STRIPE_LEN. > > > > > > > > > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > > > > > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > > > > > > > > I smell some fuzzing :) do you have the image available? I'll add it to > > > > the rest in btrfsprogs. > > > > > > Sure, it's on the way, I'll send it along with a patch for btrfsck (we > > > have to add the same validation check for superblock and chunk in > > > btrfsck.) > > > > Great! > > > > > > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > > > > --- > > > > > fs/btrfs/volumes.c | 2 +- > > > > > fs/btrfs/volumes.h | 2 +- > > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > > > > index e2b54d5..b5cb859 100644 > > > > > --- a/fs/btrfs/volumes.c > > > > > +++ b/fs/btrfs/volumes.c > > > > > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > > > > > "invalid chunk length %llu", length); > > > > > return -EIO; > > > > > } > > > > > - if (!is_power_of_2(stripe_len)) { > > > > > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > We don't need the first 'is_power_of_2' check. > > And I think we may need to have another helper, such as btrfs_check_chunk_valid(), > to cover all these (both current and future) validation checks. What do you think? Sounds good. -- 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 Fri, Apr 29, 2016 at 11:20:31AM +0800, Qu Wenruo wrote: > > > Liu Bo wrote on 2016/04/28 10:48 -0700: > >On Wed, Apr 27, 2016 at 07:33:18PM +0200, David Sterba wrote: > >>On Wed, Apr 27, 2016 at 10:23:35AM -0700, Liu Bo wrote: > >>>On Wed, Apr 27, 2016 at 06:39:03PM +0200, David Sterba wrote: > >>>>On Tue, Apr 26, 2016 at 05:53:31PM -0700, Liu Bo wrote: > >>>>>The struct 'map_lookup' uses type int for @stripe_len, while > >>>>>btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > >>>>>@stripe_len being undefined value and it can lead to 'divide error' in > >>>>> __btrfs_map_block(). > >>>>> > >>>>>This changes 'map_lookup' to use type u64 for stripe_len, also right now > >>>>>we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > >>>>>BTRFS_STRIPE_LEN. > >>>>> > >>>>>Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > >>>>>Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > >>>> > >>>>I smell some fuzzing :) do you have the image available? I'll add it to > >>>>the rest in btrfsprogs. > >>> > >>>Sure, it's on the way, I'll send it along with a patch for btrfsck (we > >>>have to add the same validation check for superblock and chunk in > >>>btrfsck.) > >> > >>Great! > >> > >>>>>Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > >>>>>--- > >>>>> fs/btrfs/volumes.c | 2 +- > >>>>> fs/btrfs/volumes.h | 2 +- > >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) > >>>>> > >>>>>diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > >>>>>index e2b54d5..b5cb859 100644 > >>>>>--- a/fs/btrfs/volumes.c > >>>>>+++ b/fs/btrfs/volumes.c > >>>>>@@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > >>>>> "invalid chunk length %llu", length); > >>>>> return -EIO; > >>>>> } > >>>>>- if (!is_power_of_2(stripe_len)) { > >>>>>+ if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > > >We don't need the first 'is_power_of_2' check. > > > >And I think we may need to have another helper, such as btrfs_check_chunk_valid(), > >to cover all these (both current and future) validation checks. What do you think? > > > >Thanks, > > > >-liubo > > > > +1 for a btrfs_check_chunk_valid(). > > And I'm OK to remove is_power_of_2 check, as it's only used for future > stripe_len. > But we only support fix STRIPE_LEN, it's OK to remove it. OK. > > BTW, did it crash btrfs-progs? Yes, not really crash progs, but btrfsck doesn't detect errors. I've made a patch and I'm testing it to make sure everything works. Thanks, -liubo > > Thanks, > Qu > > >>>> > >>>>Unfortunatelly this will break current state, as mkfs does not set the > >>>>stripe length to 64k but to 4k. But the value is otherwise ignored in > >>>>kernel. > >>> > >>>This is chunk's stripe_len, not superblock's stripe_len: > >>> > >>>make_btrfs() { > >>> ... > >>> btrfs_set_super_stripesize(&super, cfg->stripesize); --> 4096 > >>> ... > >>> btrfs_set_chunk_stripe_len(buf, chunk, 64 * 1024); > >>>} > >> > >>Oh right, and the hardcoded stripe chunk size would need to be fixed in > >>a lot more places so it's fine. Consider this > >> > >>Reviewed-by: David Sterba <dsterba@suse.com> > >> > >>and on the way to for-next. > >-- > >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
On Fri, Apr 29, 2016 at 09:59:25PM +0300, Güngör Erseymen wrote: > On 2016-04-27 at 03:53:31 +0300 (EEST), Liu Bo <bo.li.liu@oracle.com> wrote: > > The struct 'map_lookup' uses type int for @stripe_len, while > > btrfs_chunk_stripe_len() can return a u64 value, and it may end up with > > @stripe_len being undefined value and it can lead to 'divide error' in > > __btrfs_map_block(). > > > > This changes 'map_lookup' to use type u64 for stripe_len, also right now > > we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for > > BTRFS_STRIPE_LEN. > > > > Reported-by: Vegard Nossum <vegard.nossum@oracle.com> > > Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com> > > --- > > fs/btrfs/volumes.c | 2 +- > > fs/btrfs/volumes.h | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > > index e2b54d5..b5cb859 100644 > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, > > "invalid chunk length %llu", length); > > return -EIO; > > } > > - if (!is_power_of_2(stripe_len)) { > > + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { > > btrfs_err(root->fs_info, "invalid chunk stripe length: %llu", > > stripe_len); > > return -EIO; > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > > index 1939ebd..7507be7 100644 > > --- a/fs/btrfs/volumes.h > > +++ b/fs/btrfs/volumes.h > > @@ -347,7 +347,7 @@ struct map_lookup { > > u64 type; > > int io_align; > > int io_width; > > - int stripe_len; > > + u64 stripe_len; > > int sector_size; > > int num_stripes; > > int sub_stripes; > > -- > > 2.5.0 > > Type change of map->stripe_len to u64 cause build error "undefined > reference to __udivdi3" on 32bit systems. Using explicit 64 bit division > fixes it and would be better to combine into this commit for > continuity. Thanks a lot for fixing this. Thanks, -liubo > > Thanks. > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 4678f03e878e..d9ef1c4f91aa 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2860,7 +2860,7 @@ static noinline_for_stack int scrub_raid56_parity(struct scrub_ctx *sctx, > int extent_mirror_num; > int stop_loop = 0; > > - nsectors = map->stripe_len / root->sectorsize; > + nsectors = div_u64(map->stripe_len, root->sectorsize); > bitmap_len = scrub_calc_parity_bitmap_len(nsectors); > sparity = kzalloc(sizeof(struct scrub_parity) + 2 * bitmap_len, > GFP_NOFS); > > -- > Güngör Erseymen -- 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 Fri, Apr 29, 2016 at 09:59:25PM +0300, Güngör Erseymen wrote: > > --- a/fs/btrfs/volumes.h > > +++ b/fs/btrfs/volumes.h > > @@ -347,7 +347,7 @@ struct map_lookup { > > u64 type; > > int io_align; > > int io_width; > > - int stripe_len; > > + u64 stripe_len; > > int sector_size; > > int num_stripes; > > int sub_stripes; > > -- > > 2.5.0 > > Type change of map->stripe_len to u64 cause build error "undefined > reference to __udivdi3" on 32bit systems. Using explicit 64 bit division > fixes it and would be better to combine into this commit for > continuity. Done, thanks for the report. JFYI, the branches pushed to kernel.org get automated build tests so I get the email reports from kbuild eventually. -- 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/volumes.c b/fs/btrfs/volumes.c index e2b54d5..b5cb859 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6242,7 +6242,7 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key, "invalid chunk length %llu", length); return -EIO; } - if (!is_power_of_2(stripe_len)) { + if (!is_power_of_2(stripe_len) || stripe_len != BTRFS_STRIPE_LEN) { btrfs_err(root->fs_info, "invalid chunk stripe length: %llu", stripe_len); return -EIO; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 1939ebd..7507be7 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -347,7 +347,7 @@ struct map_lookup { u64 type; int io_align; int io_width; - int stripe_len; + u64 stripe_len; int sector_size; int num_stripes; int sub_stripes;
The struct 'map_lookup' uses type int for @stripe_len, while btrfs_chunk_stripe_len() can return a u64 value, and it may end up with @stripe_len being undefined value and it can lead to 'divide error' in __btrfs_map_block(). This changes 'map_lookup' to use type u64 for stripe_len, also right now we only use BTRFS_STRIPE_LEN for stripe_len, so this adds a valid checker for BTRFS_STRIPE_LEN. Reported-by: Vegard Nossum <vegard.nossum@oracle.com> Reported-by: Quentin Casasnovas <quentin.casasnovas@oracle.com> Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- fs/btrfs/volumes.c | 2 +- fs/btrfs/volumes.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)