[PATCH] md/raid10: initialize r10_bio->read_slot before use.

[PATCH] md/raid10: initialize r10_bio->read_slot before use.

[Song Liu]

From: Kevin Vigor <kvigor@gmail.com>

In __make_request() a new r10bio is allocated and passed to
raid10_read_request(). The read_slot member of the bio is not
initialized, and the raid10_read_request() uses it to index an
array. This leads to occasional panics.

Fix by initializing the field to invalid value and checking for
valid value in raid10_read_request().

Cc: stable@vger.kernel.org # v4.14+
Signed-off-by: Kevin Vigor <kvigor@gmail.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
(cherry picked from commit 93decc563637c4288380912eac0eb42fb246cc04)
---
 drivers/md/raid10.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d08d77b9674ff..419ecdd914f4c 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1120,7 +1120,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 	struct md_rdev *err_rdev = NULL;
 	gfp_t gfp = GFP_NOIO;
 
-	if (r10_bio->devs[slot].rdev) {
+	if (slot >= 0 && r10_bio->devs[slot].rdev) {
 		/*
 		 * This is an error retry, but we cannot
 		 * safely dereference the rdev in the r10_bio,
@@ -1513,6 +1513,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 	r10_bio->mddev = mddev;
 	r10_bio->sector = bio->bi_iter.bi_sector;
 	r10_bio->state = 0;
+	r10_bio->read_slot = -1;
 	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * conf->copies);
 
 	if (bio_data_dir(bio) == READ)
-- 
2.24.1

Re: [PATCH] md/raid10: initialize r10_bio->read_slot before use.

[Greg KH]

On Tue, Dec 29, 2020 at 11:40:39PM -0800, Song Liu wrote:
> From: Kevin Vigor <kvigor@gmail.com>
> 
> In __make_request() a new r10bio is allocated and passed to
> raid10_read_request(). The read_slot member of the bio is not
> initialized, and the raid10_read_request() uses it to index an
> array. This leads to occasional panics.
> 
> Fix by initializing the field to invalid value and checking for
> valid value in raid10_read_request().
> 
> Cc: stable@vger.kernel.org # v4.14+
> Signed-off-by: Kevin Vigor <kvigor@gmail.com>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> (cherry picked from commit 93decc563637c4288380912eac0eb42fb246cc04)
> ---
>  drivers/md/raid10.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Now queued up, thanks.

greg k-h

Re: [PATCH] md/raid10: initialize r10_bio->read_slot before use.

[Song Liu]

On Thu, Nov 5, 2020 at 10:08 AM Kevin Vigor <kvigor@gmail.com> wrote:
>
> In __make_request() a new r10bio is allocated and passed to
> raid10_read_request(). The read_slot member of the bio is not
> initialized, and the raid10_read_request() uses it to index an
> array. This leads to occasional panics.
>
> Fix by initializing the field to invalid value and checking for
> valid value in raid10_read_request().
>
> Signed-off-by: Kevin Vigor <kvigor@gmail.com>

Thanks for the fix!

I am having problem apply this patch, could you please rebase against

   git://git.kernel.org/pub/scm/linux/kernel/git/song/md.git  md-next

> ---
>  drivers/md/raid10.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 8a1354a08a1a..64b1306b0c4a 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1143,7 +1143,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
>         struct md_rdev *err_rdev = NULL;
>         gfp_t gfp = GFP_NOIO;
>
> -       if (r10_bio->devs[slot].rdev) {
> +       if (slot >= 0 && r10_bio->devs[slot].rdev) {

If we set default read_slot = 0, we should not need this change, right?

>                 /*
>                  * This is an error retry, but we cannot
>                  * safely dereference the rdev in the r10_bio,
> @@ -1508,6 +1508,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
>         r10_bio->mddev = mddev;
>         r10_bio->sector = bio->bi_iter.bi_sector;
>         r10_bio->state = 0;
> +       r10_bio->read_slot = -1;
>         memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * conf->copies);
>
>         if (bio_data_dir(bio) == READ)
> --
> 2.26.2
>

[PATCH] md/raid10: initialize r10_bio->read_slot before use.

[Kevin Vigor]

In __make_request() a new r10bio is allocated and passed to
raid10_read_request(). The read_slot member of the bio is not
initialized, and the raid10_read_request() uses it to index an
array. This leads to occasional panics.

Fix by initializing the field to invalid value and checking for
valid value in raid10_read_request().

Signed-off-by: Kevin Vigor <kvigor@gmail.com>
---
 drivers/md/raid10.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 8a1354a08a1a..64b1306b0c4a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1143,7 +1143,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 	struct md_rdev *err_rdev = NULL;
 	gfp_t gfp = GFP_NOIO;
 
-	if (r10_bio->devs[slot].rdev) {
+	if (slot >= 0 && r10_bio->devs[slot].rdev) {
 		/*
 		 * This is an error retry, but we cannot
 		 * safely dereference the rdev in the r10_bio,
@@ -1508,6 +1508,7 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 	r10_bio->mddev = mddev;
 	r10_bio->sector = bio->bi_iter.bi_sector;
 	r10_bio->state = 0;
+	r10_bio->read_slot = -1;
 	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * conf->copies);
 
 	if (bio_data_dir(bio) == READ)
-- 
2.26.2