On Mon, Mar 28, 2022 at 1:11 AM Xiaomeng Tong <xiam0nd.tong@xxxxxxxxx> 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@xxxxxxxxxxxxxxx > 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@xxxxxxxxx> > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@xxxxxxxxx> > --- > > 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@xxxxxxxxx/ > > --- > 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 >