* [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() @ 2010-06-21 22:14 prasanna.panchamukhi 2010-06-21 22:55 ` Neil Brown 0 siblings, 1 reply; 5+ messages in thread From: prasanna.panchamukhi @ 2010-06-21 22:14 UTC (permalink / raw) To: linux-raid; +Cc: prasanna.panchamukhi, rbecker From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> Such NULL pointer dereference can occur when the driver was fixing the read errors/bad blocks and the disk was physically removed causing a system crash. This patch check if the rcu_dereference() returns valid rdev before accessing it in fix_read_error(). Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> Signed-off-by: Rob Becker <rbecker@riverbed.com> --- drivers/md/raid10.c | 51 +++++++++++++++++++++++++++------------------------ 1 files changed, 27 insertions(+), 24 deletions(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 0372499..9556faa 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1490,31 +1490,34 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) int cur_read_error_count = 0; rdev = rcu_dereference(conf->mirrors[d].rdev); - bdevname(rdev->bdev, b); + if (rdev) { /* Check if the mirror raid device is not NULL*/ + bdevname(rdev->bdev, b); - if (test_bit(Faulty, &rdev->flags)) { - rcu_read_unlock(); - /* drive has already been failed, just ignore any - more fix_read_error() attempts */ - return; - } + if (test_bit(Faulty, &rdev->flags)) { + rcu_read_unlock(); + /* drive has already been failed, just ignore + any more fix_read_error() attempts */ + return; + } - check_decay_read_errors(mddev, rdev); - atomic_inc(&rdev->read_errors); - cur_read_error_count = atomic_read(&rdev->read_errors); - if (cur_read_error_count > max_read_errors) { - rcu_read_unlock(); - printk(KERN_NOTICE - "md/raid10:%s: %s: Raid device exceeded " - "read_error threshold " - "[cur %d:max %d]\n", - mdname(mddev), - b, cur_read_error_count, max_read_errors); - printk(KERN_NOTICE - "md/raid10:%s: %s: Failing raid " - "device\n", mdname(mddev), b); - md_error(mddev, conf->mirrors[d].rdev); - return; + check_decay_read_errors(mddev, rdev); + atomic_inc(&rdev->read_errors); + cur_read_error_count = atomic_read(&rdev->read_errors); + if (cur_read_error_count > max_read_errors) { + rcu_read_unlock(); + printk(KERN_NOTICE + "md/raid10:%s: %s: Raid device exceeded " + "read_error threshold " + "[cur %d:max %d]\n", + mdname(mddev), + b, cur_read_error_count, + max_read_errors); + printk(KERN_NOTICE + "md/raid10:%s: %s: Failing raid " + "device\n", mdname(mddev), b); + md_error(mddev, conf->mirrors[d].rdev); + return; + } } } rcu_read_unlock(); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() 2010-06-21 22:14 [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() prasanna.panchamukhi @ 2010-06-21 22:55 ` Neil Brown 2010-06-21 23:10 ` Prasanna Panchamukhi 2010-06-23 2:40 ` Prasanna S. Panchamukhi 0 siblings, 2 replies; 5+ messages in thread From: Neil Brown @ 2010-06-21 22:55 UTC (permalink / raw) To: prasanna.panchamukhi; +Cc: linux-raid, rbecker On Mon, 21 Jun 2010 15:14:35 -0700 prasanna.panchamukhi@riverbed.com wrote: > From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > > Such NULL pointer dereference can occur when the driver was fixing the > read errors/bad blocks and the disk was physically removed > causing a system crash. This patch check if the > rcu_dereference() returns valid rdev before accessing it in fix_read_error(). > > Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > Signed-off-by: Rob Becker <rbecker@riverbed.com> Thanks for the patch. However all that extra indenting is rather painful - and we already have more than we should. How about this instead? Thanks, NeilBrown diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index aa9f7b6..0334655 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1482,14 +1482,14 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) int sectors = r10_bio->sectors; mdk_rdev_t*rdev; int max_read_errors = atomic_read(&mddev->max_corr_read_errors); + int d = r10_bio->devs[r10_bio->read_slot].devnum; rcu_read_lock(); - { - int d = r10_bio->devs[r10_bio->read_slot].devnum; + rdev = rcu_dereference(conf->mirrors[d].rdev); + if (rdev) { char b[BDEVNAME_SIZE]; int cur_read_error_count = 0; - rdev = rcu_dereference(conf->mirrors[d].rdev); bdevname(rdev->bdev, b); if (test_bit(Faulty, &rdev->flags)) { @@ -1530,7 +1530,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) rcu_read_lock(); do { - int d = r10_bio->devs[sl].devnum; + d = r10_bio->devs[sl].devnum; rdev = rcu_dereference(conf->mirrors[d].rdev); if (rdev && test_bit(In_sync, &rdev->flags)) { @@ -1601,7 +1601,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) } sl = start; while (sl != r10_bio->read_slot) { - int d; + if (sl==0) sl = conf->copies; sl--; > --- > drivers/md/raid10.c | 51 +++++++++++++++++++++++++++------------------------ > 1 files changed, 27 insertions(+), 24 deletions(-) > > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > index 0372499..9556faa 100644 > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c > @@ -1490,31 +1490,34 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > int cur_read_error_count = 0; > > rdev = rcu_dereference(conf->mirrors[d].rdev); > - bdevname(rdev->bdev, b); > + if (rdev) { /* Check if the mirror raid device is not NULL*/ > + bdevname(rdev->bdev, b); > > - if (test_bit(Faulty, &rdev->flags)) { > - rcu_read_unlock(); > - /* drive has already been failed, just ignore any > - more fix_read_error() attempts */ > - return; > - } > + if (test_bit(Faulty, &rdev->flags)) { > + rcu_read_unlock(); > + /* drive has already been failed, just ignore > + any more fix_read_error() attempts */ > + return; > + } > > - check_decay_read_errors(mddev, rdev); > - atomic_inc(&rdev->read_errors); > - cur_read_error_count = atomic_read(&rdev->read_errors); > - if (cur_read_error_count > max_read_errors) { > - rcu_read_unlock(); > - printk(KERN_NOTICE > - "md/raid10:%s: %s: Raid device exceeded " > - "read_error threshold " > - "[cur %d:max %d]\n", > - mdname(mddev), > - b, cur_read_error_count, max_read_errors); > - printk(KERN_NOTICE > - "md/raid10:%s: %s: Failing raid " > - "device\n", mdname(mddev), b); > - md_error(mddev, conf->mirrors[d].rdev); > - return; > + check_decay_read_errors(mddev, rdev); > + atomic_inc(&rdev->read_errors); > + cur_read_error_count = atomic_read(&rdev->read_errors); > + if (cur_read_error_count > max_read_errors) { > + rcu_read_unlock(); > + printk(KERN_NOTICE > + "md/raid10:%s: %s: Raid device exceeded " > + "read_error threshold " > + "[cur %d:max %d]\n", > + mdname(mddev), > + b, cur_read_error_count, > + max_read_errors); > + printk(KERN_NOTICE > + "md/raid10:%s: %s: Failing raid " > + "device\n", mdname(mddev), b); > + md_error(mddev, conf->mirrors[d].rdev); > + return; > + } > } > } > rcu_read_unlock(); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() 2010-06-21 22:55 ` Neil Brown @ 2010-06-21 23:10 ` Prasanna Panchamukhi 2010-06-23 2:40 ` Prasanna S. Panchamukhi 1 sibling, 0 replies; 5+ messages in thread From: Prasanna Panchamukhi @ 2010-06-21 23:10 UTC (permalink / raw) To: Neil Brown; +Cc: linux-raid@vger.kernel.org, Rob Becker On 06/21/2010 03:55 PM, Neil Brown wrote: > On Mon, 21 Jun 2010 15:14:35 -0700 > prasanna.panchamukhi@riverbed.com wrote: > > >> From: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com> >> >> Such NULL pointer dereference can occur when the driver was fixing the >> read errors/bad blocks and the disk was physically removed >> causing a system crash. This patch check if the >> rcu_dereference() returns valid rdev before accessing it in fix_read_error(). >> >> Signed-off-by: Prasanna S. Panchamukhi<prasanna.panchamukhi@riverbed.com> >> Signed-off-by: Rob Becker<rbecker@riverbed.com> >> > Thanks for the patch. > However all that extra indenting is rather painful - and we already have more > than we should. > > How about this instead? > Looks good to me. Thanks Prasanna > Thanks, > NeilBrown > > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > index aa9f7b6..0334655 100644 > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c > @@ -1482,14 +1482,14 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > int sectors = r10_bio->sectors; > mdk_rdev_t*rdev; > int max_read_errors = atomic_read(&mddev->max_corr_read_errors); > + int d = r10_bio->devs[r10_bio->read_slot].devnum; > > rcu_read_lock(); > - { > - int d = r10_bio->devs[r10_bio->read_slot].devnum; > + rdev = rcu_dereference(conf->mirrors[d].rdev); > + if (rdev) { > char b[BDEVNAME_SIZE]; > int cur_read_error_count = 0; > > - rdev = rcu_dereference(conf->mirrors[d].rdev); > bdevname(rdev->bdev, b); > > if (test_bit(Faulty,&rdev->flags)) { > @@ -1530,7 +1530,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > > rcu_read_lock(); > do { > - int d = r10_bio->devs[sl].devnum; > + d = r10_bio->devs[sl].devnum; > rdev = rcu_dereference(conf->mirrors[d].rdev); > if (rdev&& > test_bit(In_sync,&rdev->flags)) { > @@ -1601,7 +1601,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > } > sl = start; > while (sl != r10_bio->read_slot) { > - int d; > + > if (sl==0) > sl = conf->copies; > sl--; > > >> --- >> drivers/md/raid10.c | 51 +++++++++++++++++++++++++++------------------------ >> 1 files changed, 27 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c >> index 0372499..9556faa 100644 >> --- a/drivers/md/raid10.c >> +++ b/drivers/md/raid10.c >> @@ -1490,31 +1490,34 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) >> int cur_read_error_count = 0; >> >> rdev = rcu_dereference(conf->mirrors[d].rdev); >> - bdevname(rdev->bdev, b); >> + if (rdev) { /* Check if the mirror raid device is not NULL*/ >> + bdevname(rdev->bdev, b); >> >> - if (test_bit(Faulty,&rdev->flags)) { >> - rcu_read_unlock(); >> - /* drive has already been failed, just ignore any >> - more fix_read_error() attempts */ >> - return; >> - } >> + if (test_bit(Faulty,&rdev->flags)) { >> + rcu_read_unlock(); >> + /* drive has already been failed, just ignore >> + any more fix_read_error() attempts */ >> + return; >> + } >> >> - check_decay_read_errors(mddev, rdev); >> - atomic_inc(&rdev->read_errors); >> - cur_read_error_count = atomic_read(&rdev->read_errors); >> - if (cur_read_error_count> max_read_errors) { >> - rcu_read_unlock(); >> - printk(KERN_NOTICE >> - "md/raid10:%s: %s: Raid device exceeded " >> - "read_error threshold " >> - "[cur %d:max %d]\n", >> - mdname(mddev), >> - b, cur_read_error_count, max_read_errors); >> - printk(KERN_NOTICE >> - "md/raid10:%s: %s: Failing raid " >> - "device\n", mdname(mddev), b); >> - md_error(mddev, conf->mirrors[d].rdev); >> - return; >> + check_decay_read_errors(mddev, rdev); >> + atomic_inc(&rdev->read_errors); >> + cur_read_error_count = atomic_read(&rdev->read_errors); >> + if (cur_read_error_count> max_read_errors) { >> + rcu_read_unlock(); >> + printk(KERN_NOTICE >> + "md/raid10:%s: %s: Raid device exceeded " >> + "read_error threshold " >> + "[cur %d:max %d]\n", >> + mdname(mddev), >> + b, cur_read_error_count, >> + max_read_errors); >> + printk(KERN_NOTICE >> + "md/raid10:%s: %s: Failing raid " >> + "device\n", mdname(mddev), b); >> + md_error(mddev, conf->mirrors[d].rdev); >> + return; >> + } >> } >> } >> rcu_read_unlock(); >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() 2010-06-21 22:55 ` Neil Brown 2010-06-21 23:10 ` Prasanna Panchamukhi @ 2010-06-23 2:40 ` Prasanna S. Panchamukhi 2010-06-24 0:16 ` Neil Brown 1 sibling, 1 reply; 5+ messages in thread From: Prasanna S. Panchamukhi @ 2010-06-23 2:40 UTC (permalink / raw) To: Neil Brown; +Cc: linux-raid@vger.kernel.org, Rob Becker On Mon, Jun 21, 2010 at 03:55:41PM -0700, Neil Brown wrote: > On Mon, 21 Jun 2010 15:14:35 -0700 > prasanna.panchamukhi@riverbed.com wrote: > > > From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > > > > Such NULL pointer dereference can occur when the driver was fixing the > > read errors/bad blocks and the disk was physically removed > > causing a system crash. This patch check if the > > rcu_dereference() returns valid rdev before accessing it in fix_read_error(). > > > > Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > > Signed-off-by: Rob Becker <rbecker@riverbed.com> > > Thanks for the patch. > However all that extra indenting is rather painful - and we already have more > than we should. > > How about this instead? Hi Neil, Thanks for your review, Please find the updated patch as per your suggestion below. Thanks Prasanna From c2bb6a02707335430d31dc901093108cc6046af2 Mon Sep 17 00:00:00 2001 From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> Date: Tue, 22 Jun 2010 18:59:33 -0700 Subject: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() Such NULL pointer dereference can occur when the driver was fixing the read errors/bad blocks and the disk was physically removed causing a system crash. This patch check if the rcu_dereference() returns valid rdev before accessing it in fix_read_error(). Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> Signed-off-by: Rob Becker <rbecker@riverbed.com> --- drivers/md/raid10.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c index 0372499..6d420cb 100644 --- a/drivers/md/raid10.c +++ b/drivers/md/raid10.c @@ -1482,14 +1482,14 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) int sectors = r10_bio->sectors; mdk_rdev_t*rdev; int max_read_errors = atomic_read(&mddev->max_corr_read_errors); + int d = r10_bio->devs[r10_bio->read_slot].devnum; rcu_read_lock(); - { - int d = r10_bio->devs[r10_bio->read_slot].devnum; + rdev = rcu_dereference(conf->mirrors[d].rdev); + if (rdev) { /* If rdev is not NULL */ char b[BDEVNAME_SIZE]; int cur_read_error_count = 0; - rdev = rcu_dereference(conf->mirrors[d].rdev); bdevname(rdev->bdev, b); if (test_bit(Faulty, &rdev->flags)) { @@ -1530,7 +1530,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) rcu_read_lock(); do { - int d = r10_bio->devs[sl].devnum; + d = r10_bio->devs[sl].devnum; rdev = rcu_dereference(conf->mirrors[d].rdev); if (rdev && test_bit(In_sync, &rdev->flags)) { @@ -1564,7 +1564,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) rcu_read_lock(); while (sl != r10_bio->read_slot) { char b[BDEVNAME_SIZE]; - int d; + if (sl==0) sl = conf->copies; sl--; @@ -1601,7 +1601,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) } sl = start; while (sl != r10_bio->read_slot) { - int d; + if (sl==0) sl = conf->copies; sl--; -- 1.7.0.4 > > Thanks, > NeilBrown > > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > index aa9f7b6..0334655 100644 > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c > @@ -1482,14 +1482,14 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > int sectors = r10_bio->sectors; > mdk_rdev_t*rdev; > int max_read_errors = atomic_read(&mddev->max_corr_read_errors); > + int d = r10_bio->devs[r10_bio->read_slot].devnum; > > rcu_read_lock(); > - { > - int d = r10_bio->devs[r10_bio->read_slot].devnum; > + rdev = rcu_dereference(conf->mirrors[d].rdev); > + if (rdev) { > char b[BDEVNAME_SIZE]; > int cur_read_error_count = 0; > > - rdev = rcu_dereference(conf->mirrors[d].rdev); > bdevname(rdev->bdev, b); > > if (test_bit(Faulty, &rdev->flags)) { > @@ -1530,7 +1530,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > > rcu_read_lock(); > do { > - int d = r10_bio->devs[sl].devnum; > + d = r10_bio->devs[sl].devnum; > rdev = rcu_dereference(conf->mirrors[d].rdev); > if (rdev && > test_bit(In_sync, &rdev->flags)) { > @@ -1601,7 +1601,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > } > sl = start; > while (sl != r10_bio->read_slot) { > - int d; > + > if (sl==0) > sl = conf->copies; > sl--; > > > --- > > drivers/md/raid10.c | 51 +++++++++++++++++++++++++++------------------------ > > 1 files changed, 27 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > > index 0372499..9556faa 100644 > > --- a/drivers/md/raid10.c > > +++ b/drivers/md/raid10.c > > @@ -1490,31 +1490,34 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > > int cur_read_error_count = 0; > > > > rdev = rcu_dereference(conf->mirrors[d].rdev); > > - bdevname(rdev->bdev, b); > > + if (rdev) { /* Check if the mirror raid device is not NULL*/ > > + bdevname(rdev->bdev, b); > > > > - if (test_bit(Faulty, &rdev->flags)) { > > - rcu_read_unlock(); > > - /* drive has already been failed, just ignore any > > - more fix_read_error() attempts */ > > - return; > > - } > > + if (test_bit(Faulty, &rdev->flags)) { > > + rcu_read_unlock(); > > + /* drive has already been failed, just ignore > > + any more fix_read_error() attempts */ > > + return; > > + } > > > > - check_decay_read_errors(mddev, rdev); > > - atomic_inc(&rdev->read_errors); > > - cur_read_error_count = atomic_read(&rdev->read_errors); > > - if (cur_read_error_count > max_read_errors) { > > - rcu_read_unlock(); > > - printk(KERN_NOTICE > > - "md/raid10:%s: %s: Raid device exceeded " > > - "read_error threshold " > > - "[cur %d:max %d]\n", > > - mdname(mddev), > > - b, cur_read_error_count, max_read_errors); > > - printk(KERN_NOTICE > > - "md/raid10:%s: %s: Failing raid " > > - "device\n", mdname(mddev), b); > > - md_error(mddev, conf->mirrors[d].rdev); > > - return; > > + check_decay_read_errors(mddev, rdev); > > + atomic_inc(&rdev->read_errors); > > + cur_read_error_count = atomic_read(&rdev->read_errors); > > + if (cur_read_error_count > max_read_errors) { > > + rcu_read_unlock(); > > + printk(KERN_NOTICE > > + "md/raid10:%s: %s: Raid device exceeded " > > + "read_error threshold " > > + "[cur %d:max %d]\n", > > + mdname(mddev), > > + b, cur_read_error_count, > > + max_read_errors); > > + printk(KERN_NOTICE > > + "md/raid10:%s: %s: Failing raid " > > + "device\n", mdname(mddev), b); > > + md_error(mddev, conf->mirrors[d].rdev); > > + return; > > + } > > } > > } > > rcu_read_unlock(); > ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() 2010-06-23 2:40 ` Prasanna S. Panchamukhi @ 2010-06-24 0:16 ` Neil Brown 0 siblings, 0 replies; 5+ messages in thread From: Neil Brown @ 2010-06-24 0:16 UTC (permalink / raw) To: Prasanna S. Panchamukhi; +Cc: linux-raid@vger.kernel.org, Rob Becker On Tue, 22 Jun 2010 19:40:47 -0700 "Prasanna S. Panchamukhi" <prasanna.panchamukhi@riverbed.com> wrote: > On Mon, Jun 21, 2010 at 03:55:41PM -0700, Neil Brown wrote: > > On Mon, 21 Jun 2010 15:14:35 -0700 > > prasanna.panchamukhi@riverbed.com wrote: > > > > > From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > > > > > > Such NULL pointer dereference can occur when the driver was fixing the > > > read errors/bad blocks and the disk was physically removed > > > causing a system crash. This patch check if the > > > rcu_dereference() returns valid rdev before accessing it in fix_read_error(). > > > > > > Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > > > Signed-off-by: Rob Becker <rbecker@riverbed.com> > > > > Thanks for the patch. > > However all that extra indenting is rather painful - and we already have more > > than we should. > > > > How about this instead? > > Hi Neil, > > Thanks for your review, Please find the updated patch as per your suggestion > below. Thanks. You even found a "int d;" to remove that I had missed. I've added you patch you my queue. It will go into -testing and then to Linus in due course. I have added "cc: stable@kernel.org" so that it gets included -stable releases. Thanks, NeilBrown > > Thanks > Prasanna > > >From c2bb6a02707335430d31dc901093108cc6046af2 Mon Sep 17 00:00:00 2001 > From: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > Date: Tue, 22 Jun 2010 18:59:33 -0700 > Subject: [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() > > Such NULL pointer dereference can occur when the driver was fixing the > read errors/bad blocks and the disk was physically removed > causing a system crash. This patch check if the > rcu_dereference() returns valid rdev before accessing it in fix_read_error(). > > Signed-off-by: Prasanna S. Panchamukhi <prasanna.panchamukhi@riverbed.com> > Signed-off-by: Rob Becker <rbecker@riverbed.com> > --- > drivers/md/raid10.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c > index 0372499..6d420cb 100644 > --- a/drivers/md/raid10.c > +++ b/drivers/md/raid10.c > @@ -1482,14 +1482,14 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > int sectors = r10_bio->sectors; > mdk_rdev_t*rdev; > int max_read_errors = atomic_read(&mddev->max_corr_read_errors); > + int d = r10_bio->devs[r10_bio->read_slot].devnum; > > rcu_read_lock(); > - { > - int d = r10_bio->devs[r10_bio->read_slot].devnum; > + rdev = rcu_dereference(conf->mirrors[d].rdev); > + if (rdev) { /* If rdev is not NULL */ > char b[BDEVNAME_SIZE]; > int cur_read_error_count = 0; > > - rdev = rcu_dereference(conf->mirrors[d].rdev); > bdevname(rdev->bdev, b); > > if (test_bit(Faulty, &rdev->flags)) { > @@ -1530,7 +1530,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > > rcu_read_lock(); > do { > - int d = r10_bio->devs[sl].devnum; > + d = r10_bio->devs[sl].devnum; > rdev = rcu_dereference(conf->mirrors[d].rdev); > if (rdev && > test_bit(In_sync, &rdev->flags)) { > @@ -1564,7 +1564,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > rcu_read_lock(); > while (sl != r10_bio->read_slot) { > char b[BDEVNAME_SIZE]; > - int d; > + > if (sl==0) > sl = conf->copies; > sl--; > @@ -1601,7 +1601,7 @@ static void fix_read_error(conf_t *conf, mddev_t *mddev, r10bio_t *r10_bio) > } > sl = start; > while (sl != r10_bio->read_slot) { > - int d; > + > if (sl==0) > sl = conf->copies; > sl--; ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-06-24 0:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-21 22:14 [PATCH] drivers/md: raid10: Fix null pointer dereference in fix_read_error() prasanna.panchamukhi 2010-06-21 22:55 ` Neil Brown 2010-06-21 23:10 ` Prasanna Panchamukhi 2010-06-23 2:40 ` Prasanna S. Panchamukhi 2010-06-24 0:16 ` Neil Brown
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.