libdm: report: fix incorrect memory use while using --select with --unbuffered for reporting

Under certain circumstances, the selection code can segfault:

$ vgs --select 'pv_name=~/dev/sda' --unbuffered vg0
  VG   #PV #LV #SN Attr   VSize   VFree
  vg0    6   3   0 wz--n- 744.00m 588.00m
Segmentation fault (core dumped)

The problem here is the use of --ubuffered together with regex used in
selection criteria. If the report output is not buffered, each row is
discarded as soon as it is reported. The bug is in the use of report
handle's memory - in the example above, what happens is:

  1) report handle is initialized together with its memory pool

  2) selection tree is initialized from selection criteria string
     (using the report handle's memory pool!)

    2a) this also means the regex is initialized from report handle's mem pool

  3) the object (row) is reported

    3a) any memory needed for output is intialized out of report handle's mem pool
    3b) selection criteria matching is executed - if the regex is checked the
        very first time (for the very first row reported), some more memory
        allocation happens as regex allocates internal structures "on-demand",
        it's allocating from report handle's mem pool (see also step 2a)

  4) the report output is executed

  5) the object (row) is discarded, meaning discarding all the mem pool
     memory used since step 3.

Now, with step 5) we have discarded the regex internal structures from step 3b.
When we execute reporting for another object (row), we're using the same
selection criteria (step 3b), but tihs is second time we're using the regex
and as such, it's already initialized completely. But the regex is missing the
internal structures now as they got discarded in step 5) from previous
object (row) reporting (because we're using "unbuffered" reporting).

To resolve this issue and to prevent any similar future issues where each
object/row memory is discarded after output (the unbuffered reporting) while
selection tree is global for all the object/rows, use separate memory pool
for report's selection.

This patch replaces "struct selection_node *selection_root" in struct
dm_report with new struct selection which contains both "selection_root"
and "mem" for separate mem pool used for selection.

We can change struct dm_report this way as it is not exposed via libdevmapper.

(This patch will have even more meaning for upcoming patches where selection
is used even for non-reporting commands where "internal" reporting and
selection criteria matching happens and where the internal reporting is
not buffered.)

2 files changed, 33 insertions, 17 deletions

diff --git a/WHATS_NEW b/WHATS_NEW
index 9201f3682..ff06b7b3a 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.115 -
 =====================================
+  Fix segfault while using --select with regex and --unbuffered for reporting.
   Fix automatic use of configure --enable-udev-systemd-background-jobs.
   Correctly rename active split LV with -splitmirrors for raid1.
   Add report/compact_output to lvm.conf to enable/disable compact report output.
diff --git a/libdm/libdm-report.c b/libdm/libdm-report.c
index 6d36c1e1a..e3c3d2f26 100644
--- a/libdm/libdm-report.c
+++ b/libdm/libdm-report.c
@@ -26,6 +26,11 @@
 #define RH_HEADINGS_PRINTED	0x00000200
 #define RH_ALREADY_REPORTED	0x00000400
 
+struct selection {
+	struct dm_pool *mem;
+	struct selection_node *selection_root;
+};
+
 struct dm_report {
 	struct dm_pool *mem;
 
@@ -52,7 +57,9 @@ struct dm_report {
 	/* To store caller private data */
 	void *private;
 
-	struct selection_node *selection_root;
+	/* Selection handle */
+	struct selection *selection;
+
 	/* Null-terminated array of reserved values */
 	const struct dm_report_reserved_value *reserved_values;
 };
@@ -1193,6 +1200,8 @@ struct dm_report *dm_report_init(uint32_t *report_types,
 
 void dm_report_free(struct dm_report *rh)
 {
+	if (rh->selection)
+		dm_pool_destroy(rh->selection->mem);
 	dm_pool_destroy(rh->mem);
 	dm_free(rh);
 }
@@ -1560,10 +1569,10 @@ static int _check_selection(struct dm_report *rh, struct selection_node *sn,
 
 static int _check_report_selection(struct dm_report *rh, struct dm_list *fields)
 {
-	if (!rh->selection_root)
+	if (!rh->selection)
 		return 1;
 
-	return _check_selection(rh, rh->selection_root, fields);
+	return _check_selection(rh, rh->selection->selection_root, fields);
 }
 
 int dm_report_object(struct dm_report *rh, void *object)
@@ -2410,7 +2419,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 	}
 
 	/* set up selection */
-	if (!(fs = dm_pool_zalloc(rh->mem, sizeof(struct field_selection)))) {
+	if (!(fs = dm_pool_zalloc(rh->selection->mem, sizeof(struct field_selection)))) {
 		log_error("dm_report: struct field_selection "
 			  "allocation failed for selection field %s", field_id);
 		return NULL;
@@ -2429,7 +2438,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 		memcpy(s, v, len);
 		s[len] = '\0';
 
-		fs->v.r = dm_regex_create(rh->mem, (const char **) &s, 1);
+		fs->v.r = dm_regex_create(rh->selection->mem, (const char **) &s, 1);
 		dm_free(s);
 		if (!fs->v.r) {
 			log_error("dm_report: failed to create regex "
@@ -2438,7 +2447,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 		}
 	} else {
 		/* STRING, NUMBER, SIZE or STRING_LIST */
-		if (!(s = dm_pool_alloc(rh->mem, len + 1))) {
+		if (!(s = dm_pool_alloc(rh->selection->mem, len + 1))) {
 			log_error("dm_report: dm_pool_alloc failed to store "
 				  "value for selection field %s", field_id);
 			goto error;
@@ -2450,7 +2459,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 			case DM_REPORT_FIELD_TYPE_STRING:
 				if (reserved) {
 					fs->v.s = (const char *) _get_reserved_value(reserved);
-					dm_pool_free(rh->mem, s);
+					dm_pool_free(rh->selection->mem, s);
 				} else {
 					fs->v.s = s;
 					if (_check_value_is_reserved(rh, DM_REPORT_FIELD_TYPE_STRING, fs->v.s)) {
@@ -2473,7 +2482,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 						goto error;
 					}
 				}
-				dm_pool_free(rh->mem, s);
+				dm_pool_free(rh->selection->mem, s);
 				break;
 			case DM_REPORT_FIELD_TYPE_SIZE:
 				if (reserved)
@@ -2492,7 +2501,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 						goto error;
 					}
 				}
-				dm_pool_free(rh->mem, s);
+				dm_pool_free(rh->selection->mem, s);
 				break;
 			case DM_REPORT_FIELD_TYPE_PERCENT:
 				if (reserved)
@@ -2528,7 +2537,7 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
 
 	return fs;
 error:
-	dm_pool_free(rh->mem, fs);
+	dm_pool_free(rh->selection->mem, fs);
 	return NULL;
 }
 
@@ -2730,7 +2739,7 @@ static struct selection_node *_parse_selection(struct dm_report *rh,
 			custom = NULL;
 		if (!(last = _tok_value(rh, ft, field_num, implicit,
 					last, &vs, &ve, &flags,
-					&reserved, rh->mem, custom)))
+					&reserved, rh->selection->mem, custom)))
 			goto_bad;
 	}
 
@@ -2741,7 +2750,7 @@ static struct selection_node *_parse_selection(struct dm_report *rh,
 		return_NULL;
 
 	/* create selection node */
-	if (!(sn = _alloc_selection_node(rh->mem, SEL_ITEM)))
+	if (!(sn = _alloc_selection_node(rh->selection->mem, SEL_ITEM)))
 		return_NULL;
 
 	/* add selection to selection node */
@@ -2828,7 +2837,7 @@ static struct selection_node *_parse_and_ex(struct dm_report *rh,
 	}
 
 	if (!and_sn) {
-		if (!(and_sn = _alloc_selection_node(rh->mem, SEL_AND)))
+		if (!(and_sn = _alloc_selection_node(rh->selection->mem, SEL_AND)))
 			goto error;
 	}
 	dm_list_add(&and_sn->selection.set, &n->list);
@@ -2860,7 +2869,7 @@ static struct selection_node *_parse_or_ex(struct dm_report *rh,
 	}
 
 	if (!or_sn) {
-		if (!(or_sn = _alloc_selection_node(rh->mem, SEL_OR)))
+		if (!(or_sn = _alloc_selection_node(rh->selection->mem, SEL_OR)))
 			goto error;
 	}
 	dm_list_add(&or_sn->selection.set, &n->list);
@@ -2893,7 +2902,7 @@ struct dm_report *dm_report_init_with_selection(uint32_t *report_types,
 		return NULL;
 
 	if (!selection || !selection[0]) {
-		rh->selection_root = NULL;
+		rh->selection = NULL;
 		return rh;
 	}
 
@@ -2914,7 +2923,13 @@ struct dm_report *dm_report_init_with_selection(uint32_t *report_types,
 		return rh;
 	}
 
-	if (!(root = _alloc_selection_node(rh->mem, SEL_OR)))
+	if (!(rh->selection = dm_pool_zalloc(rh->mem, sizeof(struct selection))) ||
+	    !(rh->selection->mem = dm_pool_create("report selection", 10 * 1024))) {
+		log_error("Failed to allocate report selection structure.");
+		goto bad;
+	}
+
+	if (!(root = _alloc_selection_node(rh->selection->mem, SEL_OR)))
 		goto_bad;
 
 	if (!_parse_or_ex(rh, selection, &fin, root))
@@ -2930,7 +2945,7 @@ struct dm_report *dm_report_init_with_selection(uint32_t *report_types,
 
 	_dm_report_init_update_types(rh, report_types);
 
-	rh->selection_root = root;
+	rh->selection->selection_root = root;
 	return rh;
 bad:
 	dm_report_free(rh);