Re: [PATCH] Mdmonitor: Fix segfault and improve logging

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

 



Dear Kinga, dear Oleksandr,


Am 21.03.22 um 13:32 schrieb Kinga Tanska:
From: Oleksandr Shchirskyi <oleksandr.shchirskyi@xxxxxxxxxxxxxxx>

Check that devices passed to mdmonitor are md arrays.
Also, change logging, so mdmonitor in verbose mode will report its
configuration.

Thank you for the patch. As these are two distinct changes, could you please split it into two commits?

If you could add a reproducer the segmentation fault, that’d also be great.


Kind regards,

Paul


Signed-off-by: Oleksandr Shchirskyi <oleksandr.shchirskyi@xxxxxxxxx>
Signed-off-by: Kinga Tanska <kinga.tanska@xxxxxxxxx>
---
  Monitor.c | 36 ++++++++++++++++++++++++------------
  mdadm.h   |  1 +
  mdopen.c  | 17 +++++++++++++++++
  util.c    |  2 +-
  4 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index f5412299..bd417d04 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -137,24 +137,27 @@ int Monitor(struct mddev_dev *devlist,
  	struct mddev_ident *mdlist;
  	int delay_for_event = c->delay;
- if (!mailaddr) {
+	if (!mailaddr)
  		mailaddr = conf_get_mailaddr();
-		if (mailaddr && ! c->scan)
-			pr_err("Monitor using email address \"%s\" from config file\n",
-			       mailaddr);
-	}
-	mailfrom = conf_get_mailfrom();
- if (!alert_cmd) {
+	if (!alert_cmd)
  		alert_cmd = conf_get_program();
-		if (alert_cmd && !c->scan)
-			pr_err("Monitor using program \"%s\" from config file\n",
-			       alert_cmd);
-	}
+
+	mailfrom = conf_get_mailfrom();
+
  	if (c->scan && !mailaddr && !alert_cmd && !dosyslog) {
  		pr_err("No mail address or alert command - not monitoring.\n");
  		return 1;
  	}
+
+	if (c->verbose) {
+		pr_err("Monitor is started with delay %ds\n", c->delay);
+		if (mailaddr)
+			pr_err("Monitor using email address %s\n", mailaddr);
+		if (alert_cmd)
+			pr_err("Monitor using program %s\n", alert_cmd);
+	}
+
  	info.alert_cmd = alert_cmd;
  	info.mailaddr = mailaddr;
  	info.mailfrom = mailfrom;
@@ -183,6 +186,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);
@@ -204,7 +211,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];
diff --git a/util.c b/util.c
index cdf1da24..003b2f86 100644
--- a/util.c
+++ b/util.c
@@ -268,7 +268,7 @@ int md_array_active(int fd)
  		 * GET_ARRAY_INFO doesn't provide access to the proper state
  		 * information, so fallback to a basic check for raid_disks != 0
  		 */
-		ret = ioctl(fd, GET_ARRAY_INFO, &array);
+		ret = md_get_array_info(fd, &array);
  	}
return !ret;



[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