| Message ID | 20230506012315.3370489-2-linan666@huaweicloud.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | md: bugfix of writing raid sysfs | expand |
On Fri, May 5, 2023 at 6:24 PM <linan666@huaweicloud.com> wrote: > > From: Li Nan <linan122@huawei.com> > > If we write a large number to md/bitmap_set_bits, md_bitmap_checkpage() > will return -EINVAL because "page >= bitmap->pages", but the return value > was not checked immediately in md_bitmap_get_counter() in order to set > *blocks value and slab-out-of-bounds occurs. > > Return directly if err is -EINVAL. > > Fixes: ef4256733506 ("md/bitmap: optimise scanning of empty bitmaps.") > Signed-off-by: Li Nan <linan122@huawei.com> > Reviewed-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/md/md-bitmap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c > index 920bb68156d2..0b41ef422da7 100644 > --- a/drivers/md/md-bitmap.c > +++ b/drivers/md/md-bitmap.c > @@ -1388,6 +1388,8 @@ __acquires(bitmap->lock) > int err; > > err = md_bitmap_checkpage(bitmap, page, create, 0); > + if (err == -EINVAL) > + return NULL; This logic is error prone. Since we are on it, let's fix it better. Specifically, we can move "page >= bitmap->pages" check out of md_bitmap_checkpage(). (and fix the call site in md_bitmap_resize for clustered md). Also, could you please add a mdadm test for this issue? Thanks, Song > > if (bitmap->bp[page].hijacked || > bitmap->bp[page].map == NULL) > -- > 2.31.1 >
在 2023/5/13 9:05, Song Liu 写道: > On Fri, May 5, 2023 at 6:24 PM <linan666@huaweicloud.com> wrote: >> >> From: Li Nan <linan122@huawei.com> >> >> If we write a large number to md/bitmap_set_bits, md_bitmap_checkpage() >> will return -EINVAL because "page >= bitmap->pages", but the return value >> was not checked immediately in md_bitmap_get_counter() in order to set >> *blocks value and slab-out-of-bounds occurs. >> >> Return directly if err is -EINVAL. >> >> Fixes: ef4256733506 ("md/bitmap: optimise scanning of empty bitmaps.") >> Signed-off-by: Li Nan <linan122@huawei.com> >> Reviewed-by: Yu Kuai <yukuai3@huawei.com> >> --- >> drivers/md/md-bitmap.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c >> index 920bb68156d2..0b41ef422da7 100644 >> --- a/drivers/md/md-bitmap.c >> +++ b/drivers/md/md-bitmap.c >> @@ -1388,6 +1388,8 @@ __acquires(bitmap->lock) >> int err; >> >> err = md_bitmap_checkpage(bitmap, page, create, 0); >> + if (err == -EINVAL) >> + return NULL; > > This logic is error prone. Since we are on it, let's fix it better. > Specifically, we can move "page >= bitmap->pages" check out I will check out it in v3. > of md_bitmap_checkpage(). (and fix the call site in md_bitmap_resize > for clustered md). > In md_bitmap_resize(), incoming parameters "page < bitmap->counts.page" and do not have this problem. > Also, could you please add a mdadm test for this issue? > It's my pleasure. > Thanks, > Song > >> >> if (bitmap->bp[page].hijacked || >> bitmap->bp[page].map == NULL) >> -- >> 2.31.1 >> > . Thanks for your suggesion.
diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c index 920bb68156d2..0b41ef422da7 100644 --- a/drivers/md/md-bitmap.c +++ b/drivers/md/md-bitmap.c @@ -1388,6 +1388,8 @@ __acquires(bitmap->lock) int err; err = md_bitmap_checkpage(bitmap, page, create, 0); + if (err == -EINVAL) + return NULL; if (bitmap->bp[page].hijacked || bitmap->bp[page].map == NULL)