Messages in this thread | | | Date | Fri, 7 Apr 2023 08:35:40 +0800 | Subject | Re: [PATCH] btrfs: Avoid potential integer overflow when left-shifting 32-bit int | From | Qu Wenruo <> |
| |
On 2023/4/7 03:24, Nur Hussein wrote: > In scrub_stripe(), the 32-bit signed value returned by the > nr_data_stripes(map) function call should be cast to u64 > before being shifted left by BTRFS_STRIPE_LEN_SHIFT (16), > as a cautionary measure to avoid potential overflows. We > then assign it to a u64 value anyway, so a cast before a > shift seems prudent.
I'd say it's a little overkilled.
For nr_data_stripes(), it's at most hundreds of stripes (which is already insane). Even with 16 bits left shift, we need to get 2 ** 16 stripes to overflow 32bits.
Thanks, Qu > > Signed-off-by: Nur Hussein <hussein@unixcat.org> > --- > fs/btrfs/scrub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index ccb4f58ae307..4de1665fcd52 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -2187,7 +2187,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx, > > /* Initialize @offset in case we need to go to out: label */ > get_raid56_logic_offset(physical, stripe_index, map, &offset, NULL); > - increment = nr_data_stripes(map) << BTRFS_STRIPE_LEN_SHIFT; > + increment = (u64)nr_data_stripes(map) << BTRFS_STRIPE_LEN_SHIFT; > > /* > * Due to the rotation, for RAID56 it's better to iterate each stripe
| |