Messages in this thread | | | From | Song Liu <> | Date | Thu, 7 Apr 2022 17:36:48 -0700 | Subject | Re: [PATCH v2] md: fix an incorrect NULL check in sync_sbs |
| |
On Mon, Mar 28, 2022 at 1:11 AM Xiaomeng Tong <xiam0nd.tong@gmail.com> wrote: > > The bug is here: > if (!rdev) > > The list iterator value 'rdev' will *always* be set and non-NULL > by rdev_for_each(), so it is incorrect to assume that the iterator > value will be NULL if the list is empty or no element found. > Otherwise it will bypass the NULL check and lead to invalid memory > access passing the check. > > To fix the bug, use a new variable 'iter' as the list iterator, > while using the original variable 'pdev' as a dedicated pointer to
s/pdev/rdev/
> point to the found element. > > Cc: stable@vger.kernel.org > Fixes: 2aa82191ac36c ("md-cluster: Perform a lazy update")
"Fixes" should use a hash of 12 characters (13 given here). Did checkpatch.pl complain about it?
> Acked-by: Guoqing Jiang <guoqing.jiang@linux.dev> > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com> > --- > > changes since v1: > - rephrase the subject (Guoqing Jiang) > - add Acked-by: for Guoqing Jiang > v1:https://lore.kernel.org/lkml/20220327080002.11923-1-xiam0nd.tong@gmail.com/ > > --- > drivers/md/md.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 4d38bd7dadd6..7476fc204172 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -2629,14 +2629,16 @@ static void sync_sbs(struct mddev *mddev, int nospares) > > static bool does_sb_need_changing(struct mddev *mddev) > { > - struct md_rdev *rdev; > + struct md_rdev *rdev = NULL, *iter; > struct mdp_superblock_1 *sb; > int role; > > /* Find a good rdev */ > - rdev_for_each(rdev, mddev) > - if ((rdev->raid_disk >= 0) && !test_bit(Faulty, &rdev->flags)) > + rdev_for_each(iter, mddev) > + if ((iter->raid_disk >= 0) && !test_bit(Faulty, &iter->flags)) { > + rdev = iter; > break; > + } > > /* No good device found. */ > if (!rdev) > -- > 2.17.1 >
| |