Fix a segfault when using -S|--select
This commit is contained in:
		
							parent
							
								
									fec562b760
								
							
						
					
					
						commit
						f3c0c63900
					
				
							
								
								
									
										144
									
								
								0001-lvresize-fix-check-for-mounted-and-renamed-LV-to-han.patch
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										144
									
								
								0001-lvresize-fix-check-for-mounted-and-renamed-LV-to-han.patch
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,144 @@ | |||||||
|  | From 772605c8b4c84b970d5b2a73672686e0f90ce59f Mon Sep 17 00:00:00 2001 | ||||||
|  | From: David Teigland <teigland@redhat.com> | ||||||
|  | Date: Thu, 23 Feb 2023 16:32:37 -0600 | ||||||
|  | Subject: [PATCH] lvresize: fix check for mounted and renamed LV to handle | ||||||
|  |  spaces | ||||||
|  | 
 | ||||||
|  | Replace spaces with \040 in directory paths from getmntent (mtab). | ||||||
|  | 
 | ||||||
|  | The recent commit 5374a44c5712 compares mount point directory paths | ||||||
|  | from /etc/mtab and /proc/mounts, in order to detect when a mounted | ||||||
|  | LV has been renamed.  The directory path comparison does not work | ||||||
|  | correctly when the path contains spaces because getmntent uses | ||||||
|  | ascii space chars and proc replaces spaces with \040. | ||||||
|  | 
 | ||||||
|  | (cherry picked from commit 1857eb9fe08924c2e4e5adfc322ee4a2ae5a2e67) | ||||||
|  | ---
 | ||||||
|  |  lib/device/filesystem.c   | 41 ++++++++++++++++++++++++++++++--------- | ||||||
|  |  test/shell/lvresize-fs.sh | 14 +++++++++++++ | ||||||
|  |  2 files changed, 46 insertions(+), 9 deletions(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/lib/device/filesystem.c b/lib/device/filesystem.c
 | ||||||
|  | index 9b086d8c1..2163276ed 100644
 | ||||||
|  | --- a/lib/device/filesystem.c
 | ||||||
|  | +++ b/lib/device/filesystem.c
 | ||||||
|  | @@ -234,8 +234,9 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l
 | ||||||
|  |  	char proc_fstype[FSTYPE_MAX]; | ||||||
|  |  	char proc_devpath[PATH_MAX]; | ||||||
|  |  	char proc_mntpath[PATH_MAX]; | ||||||
|  | -	char lv_mapper_path[PATH_MAX];
 | ||||||
|  | -	char mntent_mount_dir[PATH_MAX];
 | ||||||
|  | +	char mtab_mntpath[PATH_MAX];
 | ||||||
|  | +	char dm_devpath[PATH_MAX];
 | ||||||
|  | +	char tmp_path[PATH_MAX];
 | ||||||
|  |  	char *dm_name; | ||||||
|  |  	struct stat st_lv; | ||||||
|  |  	struct stat stme; | ||||||
|  | @@ -275,14 +276,36 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l
 | ||||||
|  |  			continue; | ||||||
|  |  		if (stme.st_dev != st_lv.st_rdev) | ||||||
|  |  			continue; | ||||||
|  | -		dm_strncpy(mntent_mount_dir, me->mnt_dir, sizeof(mntent_mount_dir));
 | ||||||
|  | +		dm_strncpy(mtab_mntpath, me->mnt_dir, sizeof(mtab_mntpath));
 | ||||||
|  | +		break;
 | ||||||
|  |  	} | ||||||
|  |  	endmntent(fme); | ||||||
|  |   | ||||||
|  | +	/*
 | ||||||
|  | +	 * In mtab dir path, replace each ascii space character with the
 | ||||||
|  | +	 * four characters \040 which is how /proc/mounts represents spaces.
 | ||||||
|  | +	 * The mnt dir from /etc/mtab and /proc/mounts are compared below.
 | ||||||
|  | +	 */
 | ||||||
|  | +	if (strchr(mtab_mntpath, ' ')) {
 | ||||||
|  | +		int i, j = 0;
 | ||||||
|  | +		memcpy(tmp_path, mtab_mntpath, sizeof(tmp_path));
 | ||||||
|  | +		memset(mtab_mntpath, 0, sizeof(mtab_mntpath));
 | ||||||
|  | +		for (i = 0; i < sizeof(tmp_path); i++) {
 | ||||||
|  | +			if (tmp_path[i] == ' ') {
 | ||||||
|  | +				mtab_mntpath[j++] = '\\';
 | ||||||
|  | +				mtab_mntpath[j++] = '0';
 | ||||||
|  | +				mtab_mntpath[j++] = '4';
 | ||||||
|  | +				mtab_mntpath[j++] = '0';
 | ||||||
|  | +				continue;
 | ||||||
|  | +			}
 | ||||||
|  | +			mtab_mntpath[j++] = tmp_path[i];
 | ||||||
|  | +		}
 | ||||||
|  | +	}
 | ||||||
|  | +
 | ||||||
|  |  	if (!(dm_name = dm_build_dm_name(cmd->mem, lv->vg->name, lv->name, NULL))) | ||||||
|  |  		return_0; | ||||||
|  |   | ||||||
|  | -	if ((dm_snprintf(lv_mapper_path, sizeof(lv_mapper_path), "%s/%s", dm_dir(), dm_name) < 0))
 | ||||||
|  | +	if ((dm_snprintf(dm_devpath, sizeof(dm_devpath), "%s/%s", dm_dir(), dm_name) < 0))
 | ||||||
|  |  		return_0; | ||||||
|  |   | ||||||
|  |  	if (!(fp = fopen("/proc/mounts", "r"))) | ||||||
|  | @@ -296,8 +319,8 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l
 | ||||||
|  |  		if (strcmp(fstype, proc_fstype)) | ||||||
|  |  			continue; | ||||||
|  |   | ||||||
|  | -		dir_match = !strcmp(mntent_mount_dir, proc_mntpath);
 | ||||||
|  | -		dev_match = !strcmp(lv_mapper_path, proc_devpath);
 | ||||||
|  | +		dir_match = !strcmp(mtab_mntpath, proc_mntpath);
 | ||||||
|  | +		dev_match = !strcmp(dm_devpath, proc_devpath);
 | ||||||
|  |   | ||||||
|  |  		if (dir_match) | ||||||
|  |  			found_dir++; | ||||||
|  | @@ -306,7 +329,7 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l
 | ||||||
|  |   | ||||||
|  |  		if (dir_match != dev_match) { | ||||||
|  |  			log_error("LV %s mounted at %s may have been renamed (from %s).", | ||||||
|  | -				  lv_mapper_path, proc_mntpath, proc_devpath);
 | ||||||
|  | +				  dm_devpath, proc_mntpath, proc_devpath);
 | ||||||
|  |  			renamed = 1; | ||||||
|  |  		} | ||||||
|  |  	} | ||||||
|  | @@ -327,11 +350,11 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l
 | ||||||
|  |  	} | ||||||
|  |  	/* These two are likely detected as renamed, but include checks in case. */ | ||||||
|  |  	if (found_dir > 1) { | ||||||
|  | -		log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", mntent_mount_dir);
 | ||||||
|  | +		log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", mtab_mntpath);
 | ||||||
|  |  		return 1; | ||||||
|  |  	} | ||||||
|  |  	if (found_dev > 1) { | ||||||
|  | -		log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", lv_mapper_path);
 | ||||||
|  | +		log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", dm_devpath);
 | ||||||
|  |  		return 1; | ||||||
|  |  	} | ||||||
|  |  	return 0; | ||||||
|  | diff --git a/test/shell/lvresize-fs.sh b/test/shell/lvresize-fs.sh
 | ||||||
|  | index f437652d6..de234aad5 100644
 | ||||||
|  | --- a/test/shell/lvresize-fs.sh
 | ||||||
|  | +++ b/test/shell/lvresize-fs.sh
 | ||||||
|  | @@ -30,6 +30,9 @@ which mkfs.xfs || skip
 | ||||||
|  |  mount_dir="mnt_lvresize_fs" | ||||||
|  |  mkdir -p "$mount_dir" | ||||||
|  |   | ||||||
|  | +mount_dir_space="other mnt dir"
 | ||||||
|  | +mkdir -p "$mount_dir_space"
 | ||||||
|  | +
 | ||||||
|  |  # Tests require a libblkid version that shows FSLASTBLOCK | ||||||
|  |  lvcreate -n $lv1 -L 300 $vg | ||||||
|  |  mkfs.xfs -f "$DM_DEV_DIR/$vg/$lv1" | ||||||
|  | @@ -273,6 +276,17 @@ umount "$mount_dir"
 | ||||||
|  |  lvchange -an $vg/$lv2 | ||||||
|  |  lvremove $vg/$lv2 | ||||||
|  |   | ||||||
|  | +# lvextend|lvreduce, ext4, active, mounted, mount dir with space, --fs resize, renamed LV
 | ||||||
|  | +lvcreate -n $lv -L 256M $vg
 | ||||||
|  | +mkfs.ext4 "$DM_DEV_DIR/$vg/$lv"
 | ||||||
|  | +mount "$DM_DEV_DIR/$vg/$lv" "$mount_dir_space"
 | ||||||
|  | +lvrename $vg/$lv $vg/$lv2
 | ||||||
|  | +not lvextend --fs resize -L+32M $vg/$lv2
 | ||||||
|  | +not lvreduce --fs resize -L-32M $vg/$lv2
 | ||||||
|  | +umount "$mount_dir_space"
 | ||||||
|  | +lvchange -an $vg/$lv2
 | ||||||
|  | +lvremove $vg/$lv2
 | ||||||
|  | +
 | ||||||
|  |   | ||||||
|  |  # | ||||||
|  |  # lvextend, xfs | ||||||
|  | -- 
 | ||||||
|  | 2.39.2 | ||||||
|  | 
 | ||||||
| @ -0,0 +1,70 @@ | |||||||
|  | From 268e4ee30305a73e262a7fa35f850df2728cc696 Mon Sep 17 00:00:00 2001 | ||||||
|  | From: Peter Rajnoha <prajnoha@redhat.com> | ||||||
|  | Date: Tue, 7 Mar 2023 14:45:06 +0100 | ||||||
|  | Subject: [PATCH 2/2] toollib: fix segfault if using -S|--select with | ||||||
|  |  log/report_command_log=1 setting | ||||||
|  | 
 | ||||||
|  | When we are using -S|--select for non-reporting tools while using command log | ||||||
|  | reporting (log/report_command_log=1 setting), we need to create an internal | ||||||
|  | processing handle to handle the selection itself. In this case, the internal | ||||||
|  | processing handle to execute the selection (to process the -S|--select) has | ||||||
|  | a parent handle (that is processing the actual non-reporting command). | ||||||
|  | 
 | ||||||
|  | When this parent handle exists, we can't destroy the command log report | ||||||
|  | in destroy_processing_handle as there's still the parent processing to | ||||||
|  | finish. The parent processing may still generate logs which need to be | ||||||
|  | reported in the command log report. If the command log report was | ||||||
|  | destroyed prematurely together with destroying the internal processing | ||||||
|  | handle for -S|--select, then any subsequent log request from processing | ||||||
|  | the actual command (and hence an attermpt to access the command log report) | ||||||
|  | ended up with a segfault. | ||||||
|  | 
 | ||||||
|  | See also: https://bugzilla.redhat.com/show_bug.cgi?id=2175220 | ||||||
|  | 
 | ||||||
|  | (cherry picked from commit cd14d3fcc0e03136d0cea1ab1a9edff3b8b9dbeb) | ||||||
|  | ---
 | ||||||
|  |  WHATS_NEW       |  4 ++++ | ||||||
|  |  tools/toollib.c | 15 ++++++++++++++- | ||||||
|  |  2 files changed, 18 insertions(+), 1 deletion(-) | ||||||
|  | 
 | ||||||
|  | diff --git a/WHATS_NEW b/WHATS_NEW
 | ||||||
|  | index b54a092d8..33998be7d 100644
 | ||||||
|  | --- a/WHATS_NEW
 | ||||||
|  | +++ b/WHATS_NEW
 | ||||||
|  | @@ -1,3 +1,7 @@
 | ||||||
|  | +version 2.03.20 - 
 | ||||||
|  | +====================================
 | ||||||
|  | +  Fix segfault if using -S|--select with log/report_command_log=1 setting.
 | ||||||
|  | +
 | ||||||
|  |  version 2.03.19 - 21st February 2023 | ||||||
|  |  ==================================== | ||||||
|  |    Configure supports --with-systemd-run executed from udev rules. | ||||||
|  | diff --git a/tools/toollib.c b/tools/toollib.c
 | ||||||
|  | index 194088ea6..43e628abf 100644
 | ||||||
|  | --- a/tools/toollib.c
 | ||||||
|  | +++ b/tools/toollib.c
 | ||||||
|  | @@ -2050,7 +2050,20 @@ void destroy_processing_handle(struct cmd_context *cmd, struct processing_handle
 | ||||||
|  |   | ||||||
|  |  		log_restore_report_state(cmd->cmd_report.saved_log_report_state); | ||||||
|  |   | ||||||
|  | -		if (!cmd->is_interactive) {
 | ||||||
|  | +		/*
 | ||||||
|  | +		 * Do not destroy current cmd->report_group and cmd->log_rh
 | ||||||
|  | +		 * (the log report) yet if we're running interactively
 | ||||||
|  | +		 * (== running in lvm shell) or if there's a parent handle
 | ||||||
|  | +		 * (== we're executing nested processing, like it is when
 | ||||||
|  | +		 * doing selection for parent's process_each_* processing).
 | ||||||
|  | +		 *
 | ||||||
|  | +		 * In both cases, there's still possible further processing
 | ||||||
|  | +		 * to do outside the processing covered by the handle we are
 | ||||||
|  | +		 * destroying here and for which we may still need to access
 | ||||||
|  | +		 * the log report to cover the rest of the processing.
 | ||||||
|  | +		 *
 | ||||||
|  | +		 */
 | ||||||
|  | +		if (!cmd->is_interactive && !handle->parent) {
 | ||||||
|  |  			if (!dm_report_group_destroy(cmd->cmd_report.report_group)) | ||||||
|  |  				stack; | ||||||
|  |  			cmd->cmd_report.report_group = NULL; | ||||||
|  | -- 
 | ||||||
|  | 2.39.2 | ||||||
|  | 
 | ||||||
							
								
								
									
										12
									
								
								lvm2.spec
									
									
									
									
									
								
							
							
						
						
									
										12
									
								
								lvm2.spec
									
									
									
									
									
								
							| @ -45,11 +45,12 @@ Name: lvm2 | |||||||
| Epoch: %{rhel} | Epoch: %{rhel} | ||||||
| %endif | %endif | ||||||
| Version: 2.03.19 | Version: 2.03.19 | ||||||
| Release: 1%{?dist} | Release: 2%{?dist} | ||||||
| License: GPLv2 | License: GPLv2 | ||||||
| URL: https://sourceware.org/lvm2/ | URL: https://sourceware.org/lvm2/ | ||||||
| Source0: https://sourceware.org/pub/lvm2/releases/LVM2.%{version}.tgz | Source0: https://sourceware.org/pub/lvm2/releases/LVM2.%{version}.tgz | ||||||
| #Patch1: 0001-make-generate.patch | Patch1: 0001-lvresize-fix-check-for-mounted-and-renamed-LV-to-han.patch | ||||||
|  | Patch2: 0002-toollib-fix-segfault-if-using-S-select-with-log-repo.patch | ||||||
| 
 | 
 | ||||||
| BuildRequires: make | BuildRequires: make | ||||||
| BuildRequires: gcc | BuildRequires: gcc | ||||||
| @ -103,7 +104,8 @@ or more physical volumes and creating one or more logical volumes | |||||||
| 
 | 
 | ||||||
| %prep | %prep | ||||||
| %setup -q -n LVM2.%{version} | %setup -q -n LVM2.%{version} | ||||||
| #%%patch1 -p1 -b .backup1 | %patch1 -p1 -b .backup1 | ||||||
|  | %patch2 -p1 -b .backup2 | ||||||
| 
 | 
 | ||||||
| %build | %build | ||||||
| %global _default_pid_dir /run | %global _default_pid_dir /run | ||||||
| @ -656,6 +658,10 @@ An extensive functional testsuite for LVM2. | |||||||
| %endif | %endif | ||||||
| 
 | 
 | ||||||
| %changelog | %changelog | ||||||
|  | * Wed Mar 08 2023 Marian Csontos <mcsontos@redhat.com> - 2.03.19-2 | ||||||
|  | - Fix lvresize's check for mounted and renamed LVs to handle spaces. | ||||||
|  | - Fix segfault when using -S|--select with log/report_command_log=1. | ||||||
|  | 
 | ||||||
| * Tue Feb 21 2023 Marian Csontos <mcsontos@redhat.com> - 2.03.19-1 | * Tue Feb 21 2023 Marian Csontos <mcsontos@redhat.com> - 2.03.19-1 | ||||||
| - Update to upstream version 2.03.19. | - Update to upstream version 2.03.19. | ||||||
| 
 | 
 | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user