Re: [PATCH v2 1/2] Mdmonitor: Fix segfault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/5/22 7:40 PM, Kinga Tanska wrote:
Mdadm with "--monitor" parameter requires md device
as an argument to be monitored. If given argument is
not a md device, error shall be returned. Previously
it was not checked and invalid argument caused
segmentation fault. This commit adds checking
that devices passed to mdmonitor are md devices.

Signed-off-by: Kinga Tanska <kinga.tanska@xxxxxxxxx>
Signed-off-by: Oleksandr Shchirskyi <oleksandr.shchirskyi@xxxxxxxxx>
---
  Monitor.c | 11 ++++++++++-
  mdadm.h   |  1 +
  mdopen.c  | 17 +++++++++++++++++
  3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Monitor.c b/Monitor.c
index f5412299..0b24b656 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -183,6 +183,10 @@ int Monitor(struct mddev_dev *devlist,
  				continue;
  			if (strcasecmp(mdlist->devname, "<ignore>") == 0)
  				continue;
+
+			if (!is_mddev(mdlist->devname))
+				return 1;
+
  			st = xcalloc(1, sizeof *st);
  			if (mdlist->devname[0] == '/')
  				st->devname = xstrdup(mdlist->devname);


HI Kinga,


Suddenly I realize that your test for the above change might not be complete.

mdlist->devname is permitted to be a relative path under /dev/md/, with your change such devname will be regarded as invalid by is_mddev(). Because open_mddev() only takes absolute path. Maybe is_mddev() should be called several lines behind current location.

The rested part of the patch is fine to me.


Thanks.


Coly Li

@@ -204,7 +208,12 @@ int Monitor(struct mddev_dev *devlist,
  		struct mddev_dev *dv;
for (dv = devlist; dv; dv = dv->next) {
-			struct state *st = xcalloc(1, sizeof *st);
+			struct state *st;
+
+			if (!is_mddev(dv->devname))
+				return 1;
+
+			st = xcalloc(1, sizeof *st);
  			mdlist = conf_get_ident(dv->devname);
  			st->devname = xstrdup(dv->devname);
  			st->next = statelist;
diff --git a/mdadm.h b/mdadm.h
index 8f8841d8..03151c34 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1607,6 +1607,7 @@ extern int create_mddev(char *dev, char *name, int autof, int trustworthy,
  #define	FOREIGN	2
  #define	METADATA 3
  extern int open_mddev(char *dev, int report_errors);
+extern int is_mddev(char *dev);
  extern int open_container(int fd);
  extern int metadata_container_matches(char *metadata, char *devnm);
  extern int metadata_subdev_matches(char *metadata, char *devnm);
diff --git a/mdopen.c b/mdopen.c
index 245be537..d18c9319 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -475,6 +475,23 @@ int open_mddev(char *dev, int report_errors)
  	return mdfd;
  }
+/**
+ * is_mddev() - check that file name passed is an md device.
+ * @dev: file name that has to be checked.
+ * Return: 1 if file passed is an md device, 0 if not.
+ */
+int is_mddev(char *dev)
+{
+	int fd = open_mddev(dev, 1);
+
+	if (fd >= 0) {
+		close(fd);
+		return 1;
+	}
+
+	return 0;
+}
+
  char *find_free_devnm(int use_partitions)
  {
  	static char devnm[32];





[Index of Archives]     [Linux RAID Wiki]     [ATA RAID]     [Linux SCSI Target Infrastructure]     [Linux Block]     [Linux IDE]     [Linux SCSI]     [Linux Hams]     [Device Mapper]     [Device Mapper Cryptographics]     [Kernel]     [Linux Admin]     [Linux Net]     [GFS]     [RPM]     [git]     [Yosemite Forum]


  Powered by Linux