* [PATCH 1/1] md: Free resources in __md_stop
@ 2023-02-22 3:59 Xiao Ni
2023-02-23 1:27 ` Song Liu
0 siblings, 1 reply; 2+ messages in thread
From: Xiao Ni @ 2023-02-22 3:59 UTC (permalink / raw)
To: song; +Cc: yukuai1, linux-raid, yi.zhang, yangerkun, heinzm, ncroxon
If md_run() fails after ->active_io is initialized, then percpu_ref_exit
is called in error path. However, later md_free_disk will call
percpu_ref_exit again which leads to a panic because of null pointer
dereference. It can also trigger this bug when resources are initialized
but are freed in error path, then will be freed again in md_free_disk.
BUG: kernel NULL pointer dereference, address: 0000000000000038
Oops: 0000 [#1] PREEMPT SMP
Workqueue: md_misc mddev_delayed_delete
RIP: 0010:free_percpu+0x110/0x630
Call Trace:
<TASK>
__percpu_ref_exit+0x44/0x70
percpu_ref_exit+0x16/0x90
md_free_disk+0x2f/0x80
disk_release+0x101/0x180
device_release+0x84/0x110
kobject_put+0x12a/0x380
kobject_put+0x160/0x380
mddev_delayed_delete+0x19/0x30
process_one_work+0x269/0x680
worker_thread+0x266/0x640
kthread+0x151/0x1b0
ret_from_fork+0x1f/0x30
For creating raid device, md raid calls do_md_run->md_run, dm raid calls
md_run. We alloc those memory in md_run. For stopping raid device, md raid
calls do_md_stop->__md_stop, dm raid calls md_stop->__md_stop. So we can
free those memory resources in __md_stop.
Fixes: 72adae23a72c ("md: Change active_io to percpu")
Reported-and-tested-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Xiao Ni <xni@redhat.com>
---
drivers/md/md.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 927a43db5dfb..f5480778e2f7 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6256,6 +6256,11 @@ static void __md_stop(struct mddev *mddev)
mddev->to_remove = &md_redundancy_group;
module_put(pers->owner);
clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
+
+ percpu_ref_exit(&mddev->writes_pending);
+ percpu_ref_exit(&mddev->active_io);
+ bioset_exit(&mddev->bio_set);
+ bioset_exit(&mddev->sync_set);
}
void md_stop(struct mddev *mddev)
@@ -6265,10 +6270,6 @@ void md_stop(struct mddev *mddev)
*/
__md_stop_writes(mddev);
__md_stop(mddev);
- percpu_ref_exit(&mddev->writes_pending);
- percpu_ref_exit(&mddev->active_io);
- bioset_exit(&mddev->bio_set);
- bioset_exit(&mddev->sync_set);
}
EXPORT_SYMBOL_GPL(md_stop);
@@ -7839,11 +7840,6 @@ static void md_free_disk(struct gendisk *disk)
{
struct mddev *mddev = disk->private_data;
- percpu_ref_exit(&mddev->writes_pending);
- percpu_ref_exit(&mddev->active_io);
- bioset_exit(&mddev->bio_set);
- bioset_exit(&mddev->sync_set);
-
mddev_free(mddev);
}
--
2.32.0 (Apple Git-132)
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/1] md: Free resources in __md_stop
2023-02-22 3:59 [PATCH 1/1] md: Free resources in __md_stop Xiao Ni
@ 2023-02-23 1:27 ` Song Liu
0 siblings, 0 replies; 2+ messages in thread
From: Song Liu @ 2023-02-23 1:27 UTC (permalink / raw)
To: Xiao Ni; +Cc: yukuai1, linux-raid, yi.zhang, yangerkun, heinzm, ncroxon
On Tue, Feb 21, 2023 at 7:59 PM Xiao Ni <xni@redhat.com> wrote:
>
> If md_run() fails after ->active_io is initialized, then percpu_ref_exit
> is called in error path. However, later md_free_disk will call
> percpu_ref_exit again which leads to a panic because of null pointer
> dereference. It can also trigger this bug when resources are initialized
> but are freed in error path, then will be freed again in md_free_disk.
>
> BUG: kernel NULL pointer dereference, address: 0000000000000038
> Oops: 0000 [#1] PREEMPT SMP
> Workqueue: md_misc mddev_delayed_delete
> RIP: 0010:free_percpu+0x110/0x630
> Call Trace:
> <TASK>
> __percpu_ref_exit+0x44/0x70
> percpu_ref_exit+0x16/0x90
> md_free_disk+0x2f/0x80
> disk_release+0x101/0x180
> device_release+0x84/0x110
> kobject_put+0x12a/0x380
> kobject_put+0x160/0x380
> mddev_delayed_delete+0x19/0x30
> process_one_work+0x269/0x680
> worker_thread+0x266/0x640
> kthread+0x151/0x1b0
> ret_from_fork+0x1f/0x30
>
> For creating raid device, md raid calls do_md_run->md_run, dm raid calls
> md_run. We alloc those memory in md_run. For stopping raid device, md raid
> calls do_md_stop->__md_stop, dm raid calls md_stop->__md_stop. So we can
> free those memory resources in __md_stop.
>
> Fixes: 72adae23a72c ("md: Change active_io to percpu")
> Reported-and-tested-by: Yu Kuai <yukuai3@huawei.com>
> Signed-off-by: Xiao Ni <xni@redhat.com>
Applied to md-fixes. Thanks!
Song
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-02-23 1:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22 3:59 [PATCH 1/1] md: Free resources in __md_stop Xiao Ni
2023-02-23 1:27 ` Song Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.