On Sat, Feb 24, 2018 at 12:05:56PM +0800, Yufen Yu wrote:
> In handle_write_finished(), if r1_bio->bios[m] != NULL, it thinks
> the corresponding conf->mirrors[m].rdev is also not NULL. But, it
> is not always true.
>
> Even if some io hold replacement rdev(i.e. rdev->nr_pending.count > 0),
> raid1_remove_disk() can also set the rdev as NULL. That means,
> bios[m] != NULL, but mirrors[m].rdev is NULL, resulting in NULL
> pointer dereference in handle_write_finished and sync_request_write.
>
> This patch can fix BUGs as follows:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000140
> IP: [<ffffffff815bbbbd>] raid1d+0x2bd/0xfc0
> PGD 12ab52067 PUD 12f587067 PMD 0
> Oops: 0000 [#1] SMP
> CPU: 1 PID: 2008 Comm: md3_raid1 Not tainted 4.1.44+ #130
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014
> Call Trace:
> ? schedule+0x37/0x90
> ? prepare_to_wait_event+0x83/0xf0
> md_thread+0x144/0x150
> ? wake_atomic_t_function+0x70/0x70
> ? md_start_sync+0xf0/0xf0
> kthread+0xd8/0xf0
> ? kthread_worker_fn+0x160/0x160
> ret_from_fork+0x42/0x70
> ? kthread_worker_fn+0x160/0x160
>
> BUG: unable to handle kernel NULL pointer dereference at 00000000000000b8
> IP: sync_request_write+0x9e/0x980
> PGD 800000007c518067 P4D 800000007c518067 PUD 8002b067 PMD 0
> Oops: 0000 [#1] SMP PTI
> CPU: 24 PID: 2549 Comm: md3_raid1 Not tainted 4.15.0+ #118
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014
> Call Trace:
> ? sched_clock+0x5/0x10
> ? sched_clock_cpu+0xc/0xb0
> ? flush_pending_writes+0x3a/0xd0
> ? pick_next_task_fair+0x4d5/0x5f0
> ? __switch_to+0xa2/0x430
> raid1d+0x65a/0x870
> ? find_pers+0x70/0x70
> ? find_pers+0x70/0x70
> ? md_thread+0x11c/0x160
> md_thread+0x11c/0x160
> ? finish_wait+0x80/0x80
> kthread+0x111/0x130
> ? kthread_create_worker_on_cpu+0x70/0x70
> ? do_syscall_64+0x6f/0x190
> ? SyS_exit_group+0x10/0x10
> ret_from_fork+0x35/0x40
>
> Reviewed-by: NeilBrown <neilb@xxxxxxxx>
> Signed-off-by: Yufen Yu <yuyufen@xxxxxxxxxx>
Applied, thanks!
> ---
> drivers/md/raid1.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 6df398e3a008..1c5a72d3af32 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1798,6 +1798,17 @@ static int raid1_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
> struct md_rdev *repl =
> conf->mirrors[conf->raid_disks + number].rdev;
> freeze_array(conf, 0);
> + if (atomic_read(&repl->nr_pending)) {
> + /* It means that some queued IO of retry_list
> + * hold repl. Thus, we cannot set replacement
> + * as NULL, avoiding rdev NULL pointer
> + * dereference in sync_request_write and
> + * handle_write_finished.
> + */
> + err = -EBUSY;
> + unfreeze_array(conf);
> + goto abort;
> + }
> clear_bit(Replacement, &repl->flags);
> p->rdev = repl;
> conf->mirrors[conf->raid_disks + number].rdev = NULL;
> --
> 2.13.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
