metadata: add internal error if PV has no existing device attached during find_pv_in_vg

find_pv_in_vg fn iterates over the list of PVs covered by the VG and
each PV's pvl->pv->dev is compared with device acquired from device
cache. However, in case pvl->pv->dev is NULL as well as device cache
returns NULL (e.g. when device is filtered), we'll get incorrect match
and the code calling find_pv_in_vg uses incorrect PV (as it thinks
it's the exact PV with the pv_name). The INTERNAL_ERROR covers this
situation and errors out immediately.

1 files changed, 26 insertions, 2 deletions

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 8443f94c4..681c2dd0a 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1778,10 +1778,34 @@ struct pv_list *find_pv_in_vg(const struct volume_group *vg,
 			       const char *pv_name)
 {
 	struct pv_list *pvl;
+	struct device *pv_dev, *cached_dev;
 
-	dm_list_iterate_items(pvl, &vg->pvs)
-		if (pvl->pv->dev == dev_cache_get(pv_name, vg->cmd->filter))
+	dm_list_iterate_items(pvl, &vg->pvs) {
+		pv_dev = pvl->pv->dev;
+		cached_dev = dev_cache_get(pv_name, vg->cmd->filter);
+		if (!pv_dev) {
+			/*
+			 * pv_dev can't be NULL here!
+			 * We have to catch this situation earlier in the
+			 * code if this internal error is hit. Otherwise,
+			 * there's a possibility that pv_dev will match
+			 * cached_dev in case both are NULL.
+			 *
+			 * NULL cached_dev may happen in case this device
+			 * is filtered and NULL pv_dev may happen if the
+			 * device is missing!
+			 *
+			 * If such incorrect match was hit, simply incorrect
+			 * PV would be processed. This really needs to be
+			 * handled earlier in the code in that case.
+			 */
+			log_error(INTERNAL_ERROR "find_pv_in_vg: PV that is not "
+				  "bound to any existing device found");
+			return NULL;
+		}
+		if (pv_dev == cached_dev)
 			return pvl;
+	}
 
 	return NULL;
 }