[PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

[PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

[miaoguanqin]

We found PVID file leakage problem when exec the following test;
  pvcreate /dev/sdb
  vgcreate -s 1M docker /dev/sdb
  lvcreate -L 10M docker
  pvscan --cache --listvg --checkcomplete --vgonline --udevoutput 
--journal=output /dev/sdb
  vgremove -ff docker

pvcreate operation generates a new PVID, which is used to create PVID 
pvid file
when exec pvcan. However vgremove does not delete the old PVID file.

Here, we will delete all PVID files of each pv in current vg, when exec 
vgremove.

Signed-off-by: miaoguanqin<miaopguanqin@huawei.com
---
  lib/metadata/metadata.c | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 1cda1888f..75233d120 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -38,6 +38,30 @@
  #include <time.h>
  #include <math.h>

+
+static const char *_pvs_online_dir = DEFAULT_RUN_DIR "/pvs_online";
+
+static int remove_pvid_file(struct physical_volume *pv)
+{
+	char pvid[ID_LEN + 1] __attribute__((aligned(8))) = { 0 };
+	char path[PATH_MAX] __attribute__((aligned(8))) = { 0 };
+
+	memcpy(pvid, &pv->id.uuid, ID_LEN);
+
+	if (dm_snprintf(path, sizeof(path), "%s/%s", _pvs_online_dir, pvid) < 0) {
+		log_warn("WARNING:pvid file path is %s", path);
+		return 0;
+	}
+
+	log_warn("unlink pvid file, path is %s", path);
+
+	if (unlink(path) && (errno != ENOENT)) {
+		log_warn("WARNING:unlink pvid file path error, path is %s", path);
+		return 0;
+	}
+	return 1;
+}
+
  static struct physical_volume *_pv_read(struct cmd_context *cmd,
  					const struct format_type *fmt,
  					struct volume_group *vg,
@@ -665,6 +689,15 @@ int vg_remove_direct(struct volume_group *vg)
  				  " from volume group \"%s\"",
  				  pv_dev_name(pv), vg->name);
  			ret = 0;
+			continue;
+		}
+
+		/* Remove pvid files */
+		if(!remove_pvid_file(pv)){
+			log_error("Failed to remove pvid files \"%s\""
+				  " from volume group \"%s\"",
+				  _pvs_online_dir, vg->name);
+			ret = 0;
  		}
  	}

-- 
2.33.0

[PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

[David Teigland]

On Thu, Sep 08, 2022 at 10:03:40PM +0800, miaoguanqin wrote:
> We found PVID file leakage problem when exec the following test;
>  pvcreate /dev/sdb
>  vgcreate -s 1M docker /dev/sdb
>  lvcreate -L 10M docker
>  pvscan --cache --listvg --checkcomplete --vgonline --udevoutput
> --journal=output /dev/sdb
>  vgremove -ff docker
> 
> pvcreate operation generates a new PVID, which is used to create PVID pvid
> file
> when exec pvcan. However vgremove does not delete the old PVID file.
> 
> Here, we will delete all PVID files of each pv in current vg, when exec
> vgremove.

Hi, these files are located in tmpfs, so they will disappear automatically
when the system reboots.  Is there a reason that you want remove them in
vgremove instead of waiting for reboot to remove them?

Dave

[PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

[liuzhiqiang]

Thanks for your reply.
/run dir is mounted as tmpfs type, which has a setting inode number by systemd service. If the number of leakage pvid files increases with time, it may cause Deny of Service for other apps because cannot create files in /run.




????David Teigland <teigland@redhat.com>
????LVM2 development <lvm-devel@redhat.com>
????zkabelac <zkabelac@redhat.com>;linfeilong <linfeilong@huawei.com>;wuguanghao <wuguanghao3@huawei.com>;liuzhiqiang (I) <liuzhiqiang26@huawei.com>
????2022-09-09 22:27:45
????Re: [lvm-devel] [PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

On Thu, Sep 08, 2022 at 10:03:40PM +0800, miaoguanqin wrote:
> We found PVID file leakage problem when exec the following test;
> pvcreate /dev/sdb
> vgcreate -s 1M docker /dev/sdb
> lvcreate -L 10M docker
> pvscan --cache --listvg --checkcomplete --vgonline --udevoutput
> --journal=output /dev/sdb
> vgremove -ff docker
>
> pvcreate operation generates a new PVID, which is used to create PVID pvid
> file
> when exec pvcan. However vgremove does not delete the old PVID file.
>
> Here, we will delete all PVID files of each pv in current vg, when exec
> vgremove.

Hi, these files are located in tmpfs, so they will disappear automatically
when the system reboots. Is there a reason that you want remove them in
vgremove instead of waiting for reboot to remove them?

Dave


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20220910/67b53b3a/attachment.htm>

[PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online

[David Teigland]

Hi, a patch for this is here:
https://sourceware.org/git/?p=lvm2.git;a=commit;h=0887896847807e159a70edc5ac92a4030c13923a

Dave

On Sat, Sep 10, 2022 at 01:56:59AM +0000, liuzhiqiang (I) wrote:
> Thanks for your reply.
> /run dir is mounted as tmpfs type, which has a setting inode number by systemd service. If the number of leakage pvid files increases with time, it may cause Deny of Service for other apps because cannot create files in /run.
> 
> ????David Teigland <teigland@redhat.com>
> ????LVM2 development <lvm-devel@redhat.com>
> ????zkabelac <zkabelac@redhat.com>;linfeilong <linfeilong@huawei.com>;wuguanghao <wuguanghao3@huawei.com>;liuzhiqiang (I) <liuzhiqiang26@huawei.com>
> ????2022-09-09 22:27:45
> ????Re: [lvm-devel] [PATCH] vgremove: PVID file leakage in /run/lvm/pvs_online
> 
> On Thu, Sep 08, 2022 at 10:03:40PM +0800, miaoguanqin wrote:
> > We found PVID file leakage problem when exec the following test;
> > pvcreate /dev/sdb
> > vgcreate -s 1M docker /dev/sdb
> > lvcreate -L 10M docker
> > pvscan --cache --listvg --checkcomplete --vgonline --udevoutput
> > --journal=output /dev/sdb
> > vgremove -ff docker
> >
> > pvcreate operation generates a new PVID, which is used to create PVID pvid
> > file
> > when exec pvcan. However vgremove does not delete the old PVID file.
> >
> > Here, we will delete all PVID files of each pv in current vg, when exec
> > vgremove.
> 
> Hi, these files are located in tmpfs, so they will disappear automatically
> when the system reboots. Is there a reason that you want remove them in
> vgremove instead of waiting for reboot to remove them?