On 16:37 08/04, Xiaomeng Tong 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 'rdev' as a dedicated pointer to > point to the found element. > > Cc: stable@xxxxxxxxxxxxxxx > Fixes: 2aa82191ac36 ("md-cluster: Perform a lazy update") > Acked-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx> > Signed-off-by: Xiaomeng Tong <xiam0nd.tong@xxxxxxxxx> Also safeguards from reading sb from a faulty device if all devices are faulty. Acked-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> > v2:https://lore.kernel.org/lkml/20220328081127.26148-1-xiam0nd.tong@xxxxxxxxx/ > 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 > -- Goldwyn