Messages in this thread | | | Date | Sat, 27 May 2023 09:28:42 +0800 | Subject | Re: [PATCH v2 1/4] md/raid10: fix null-ptr-deref of mreplace in raid10_sync_request | From | Li Nan <> |
| |
在 2023/5/27 5:38, Song Liu 写道: > On Fri, May 26, 2023 at 12:47 AM <linan666@huaweicloud.com> wrote: >> >> From: Li Nan <linan122@huawei.com> >> >> need_replace will be set to 1 if no-Faulty mreplace exists, and mreplace >> will be deref later. However, the latter check of mreplace might set >> mreplace to NULL, null-ptr-deref occurs if need_replace is 1 at this time. >> >> Fix it by merging two checks into one. And replace 'need_replace' with >> 'mreplace' because their values are always the same. >> >> Fixes: ee37d7314a32 ("md/raid10: Fix raid10 replace hang when new added disk faulty") >> Signed-off-by: Li Nan <linan122@huawei.com> >> --- >> drivers/md/raid10.c | 13 +++++-------- >> 1 file changed, 5 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c >> index 4fcfcb350d2b..e21502c03b45 100644 >> --- a/drivers/md/raid10.c >> +++ b/drivers/md/raid10.c >> @@ -3438,7 +3438,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, >> int must_sync; >> int any_working; >> int need_recover = 0; >> - int need_replace = 0; >> struct raid10_info *mirror = &conf->mirrors[i]; >> struct md_rdev *mrdev, *mreplace; >> >> @@ -3451,10 +3450,10 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, >> !test_bit(In_sync, &mrdev->flags)) >> need_recover = 1; >> if (mreplace != NULL && >> - !test_bit(Faulty, &mreplace->flags)) >> - need_replace = 1; >> + test_bit(Faulty, &mreplace->flags)) >> + mreplace = NULL; >> >> - if (!need_recover && !need_replace) { >> + if (!need_recover && !mreplace) { >> rcu_read_unlock(); >> continue; >> } >> @@ -3470,8 +3469,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr, >> rcu_read_unlock(); >> continue; >> } > > To make sure I understand the issue correctly: > > The null-ptr-deref only happens when the Faulty bit was set after the > last check and before this check below, right? >
Yes. I will improve log in next version.
-- Thanks, Nan
| |