public inbox for linux-raid@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Mdmonitor: Fix segfault and improve logging
@ 2022-01-20 12:32 Kinga Tanska
  0 siblings, 0 replies; 7+ messages in thread
From: Kinga Tanska @ 2022-01-20 12:32 UTC (permalink / raw)
  To: linux-raid; +Cc: jes

From: Oleksandr Shchirskyi <oleksandr.shchirskyi@linux.intel.com>

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

Signed-off-by: Oleksandr Shchirskyi <oleksandr.shchirskyi@intel.com>
Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
---
 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;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] Mdmonitor: Fix segfault and improve logging
@ 2022-03-21 12:32 Kinga Tanska
  2022-03-21 12:45 ` Paul Menzel
  0 siblings, 1 reply; 7+ messages in thread
From: Kinga Tanska @ 2022-03-21 12:32 UTC (permalink / raw)
  To: linux-raid; +Cc: jes, colyli

From: Oleksandr Shchirskyi <oleksandr.shchirskyi@linux.intel.com>

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

Signed-off-by: Oleksandr Shchirskyi <oleksandr.shchirskyi@intel.com>
Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
---
 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;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Mdmonitor: Fix segfault and improve logging
  2022-03-21 12:32 [PATCH] Mdmonitor: Fix segfault and improve logging Kinga Tanska
@ 2022-03-21 12:45 ` Paul Menzel
  2022-03-23  9:02   ` Tanska, Kinga
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2022-03-21 12:45 UTC (permalink / raw)
  To: Kinga Tanska; +Cc: linux-raid, jes, colyli

Dear Kinga, dear Oleksandr,


Am 21.03.22 um 13:32 schrieb Kinga Tanska:
> From: Oleksandr Shchirskyi <oleksandr.shchirskyi@linux.intel.com>
> 
> 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@intel.com>
> Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
> ---
>   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;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Mdmonitor: Fix segfault and improve logging
  2022-03-21 12:45 ` Paul Menzel
@ 2022-03-23  9:02   ` Tanska, Kinga
  2022-03-23 10:31     ` Paul Menzel
  0 siblings, 1 reply; 7+ messages in thread
From: Tanska, Kinga @ 2022-03-23  9:02 UTC (permalink / raw)
  To: Paul Menzel, Kinga Tanska; +Cc: linux-raid, jes, colyli

Hi,

I will send splitted patches in next messages. There are steps to reproduce:

1. Stop mdmonitor instance e.g.

# systemctl stop mdmonitor

2. Run mdmonitor for non-md device e.g.

# mdadm --monitor /dev/nvme1n1


Call trace:

#0  0x00007ffff7617518 in __strcpy_sse2_unaligned () from 
/usr/lib64/libc.so.6
#1  0x000000000042bc9e in check_array ()
#2  0x000000000042c7af in Monitor ()
#3  0x0000000000406edc in main ()


Regards,

Kinga Tanska


On 3/21/2022 1:45 PM, Paul Menzel wrote:
Dear Kinga, dear Oleksandr,


Am 21.03.22 um 13:32 schrieb Kinga Tanska:
> From: Oleksandr Shchirskyi <oleksandr.shchirskyi@linux.intel.com>
>
> 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@intel.com>
> Signed-off-by: Kinga Tanska <kinga.tanska@intel.com>
> ---
>   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;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Mdmonitor: Fix segfault and improve logging
  2022-03-23  9:02   ` Tanska, Kinga
@ 2022-03-23 10:31     ` Paul Menzel
  2022-04-04 12:39       ` Tanska, Kinga
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2022-03-23 10:31 UTC (permalink / raw)
  To: Kinga Tanska; +Cc: Kinga Tanska, linux-raid, jes, colyli

Dear Kinga,


Am 23.03.22 um 10:02 schrieb Tanska, Kinga:

> I will send splitted patches in next messages.

Thank you.

> There are steps to reproduce:
> 
> 1. Stop mdmonitor instance e.g.
> 
> # systemctl stop mdmonitor
> 
> 2. Run mdmonitor for non-md device e.g.
> 
> # mdadm --monitor /dev/nvme1n1
> 
> 
> Call trace:
> 
> #0  0x00007ffff7617518 in __strcpy_sse2_unaligned () from 
> /usr/lib64/libc.so.6
> #1  0x000000000042bc9e in check_array ()
> #2  0x000000000042c7af in Monitor ()
> #3  0x0000000000406edc in main ()

I am unable to reproduce it with mdadm 4.1:

     $ sudo strace mdadm --monitor /dev/sda
     execve("/usr/bin/mdadm", ["mdadm", "--monitor", "/dev/sda"], 
0x7ffc8b00e880 /* 14 vars */) = 0
     […]
     openat(AT_FDCWD, "/dev/sda", O_RDONLY)  = 3
fstat(3, {st_mode=S_IFBLK|0660, st_rdev=makedev(0x8, 0), ...}) = 0
readlink("/sys/dev/block/8:0", "../../devices/pci0000:00/0000:00"..., 
199) = 91
     close(3)                                = 0
     pselect6(1, NULL, NULL, [], {tv_sec=1000, tv_nsec=0}, NULL) = ? 
ERESTARTNOHAND (To be restarted if no handler)
     --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
     pselect6(1, NULL, NULL, [], {tv_sec=999, tv_nsec=568020697}, NULL) 
= ? ERESTARTNOHAND (To be restarted if no handler)
     --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
pselect6(1, NULL, NULL, [], {tv_sec=999, tv_nsec=521836826}, NULL

Did a specific commit introduce the problem? If so, it’d be great if you 
added a Fixes line.

[…]


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Mdmonitor: Fix segfault and improve logging
  2022-03-23 10:31     ` Paul Menzel
@ 2022-04-04 12:39       ` Tanska, Kinga
  2022-04-04 12:54         ` Paul Menzel
  0 siblings, 1 reply; 7+ messages in thread
From: Tanska, Kinga @ 2022-04-04 12:39 UTC (permalink / raw)
  To: Paul Menzel; +Cc: Kinga Tanska, linux-raid, jes, colyli

Hello Paul,


I've tried to reproduce it with mdadm 4.1 and mdadm 4.2 and both apps 
exit with

segmentation fault. This patch is not a fix, it adds checking if device 
is md array.

It hasn't been checked before in mdmonitor, so segmentation fault should 
appear.


Regards,

Kinga


On 3/23/2022 11:31 AM, Paul Menzel wrote:
Dear Kinga,


Am 23.03.22 um 10:02 schrieb Tanska, Kinga:

> I will send splitted patches in next messages.

Thank you.

> There are steps to reproduce:
>
> 1. Stop mdmonitor instance e.g.
>
> # systemctl stop mdmonitor
>
> 2. Run mdmonitor for non-md device e.g.
>
> # mdadm --monitor /dev/nvme1n1
>
>
> Call trace:
>
> #0  0x00007ffff7617518 in __strcpy_sse2_unaligned () from 
> /usr/lib64/libc.so.6
> #1  0x000000000042bc9e in check_array ()
> #2  0x000000000042c7af in Monitor ()
> #3  0x0000000000406edc in main ()

I am unable to reproduce it with mdadm 4.1:

     $ sudo strace mdadm --monitor /dev/sda
     execve("/usr/bin/mdadm", ["mdadm", "--monitor", "/dev/sda"], 
0x7ffc8b00e880 /* 14 vars */) = 0
     […]
     openat(AT_FDCWD, "/dev/sda", O_RDONLY)  = 3
fstat(3, {st_mode=S_IFBLK|0660, st_rdev=makedev(0x8, 0), ...}) = 0
readlink("/sys/dev/block/8:0", "../../devices/pci0000:00/0000:00"..., 
199) = 91
     close(3)                                = 0
     pselect6(1, NULL, NULL, [], {tv_sec=1000, tv_nsec=0}, NULL) = ? 
ERESTARTNOHAND (To be restarted if no handler)
     --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
     pselect6(1, NULL, NULL, [], {tv_sec=999, tv_nsec=568020697}, NULL) 
= ? ERESTARTNOHAND (To be restarted if no handler)
     --- SIGWINCH {si_signo=SIGWINCH, si_code=SI_KERNEL} ---
pselect6(1, NULL, NULL, [], {tv_sec=999, tv_nsec=521836826}, NULL

Did a specific commit introduce the problem? If so, it’d be great if you 
added a Fixes line.

[…]


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Mdmonitor: Fix segfault and improve logging
  2022-04-04 12:39       ` Tanska, Kinga
@ 2022-04-04 12:54         ` Paul Menzel
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Menzel @ 2022-04-04 12:54 UTC (permalink / raw)
  To: Kinga Tanska; +Cc: Kinga Tanska, linux-raid, jes, colyli

Dear Kinga,


Am 04.04.22 um 14:39 schrieb Tanska, Kinga:

> I've tried to reproduce it with mdadm 4.1 and mdadm 4.2 and both apps
>  exit with segmentation fault. This patch is not a fix, it adds
> checking if device is md array.
> 
> It hasn't been checked before in mdmonitor, so segmentation fault
> should appear.
Understood, still it’d be nice to have a way to reproduce this. So 
please elaborate in the commit message, and give more details about your 
test setup.


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-04-04 12:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-21 12:32 [PATCH] Mdmonitor: Fix segfault and improve logging Kinga Tanska
2022-03-21 12:45 ` Paul Menzel
2022-03-23  9:02   ` Tanska, Kinga
2022-03-23 10:31     ` Paul Menzel
2022-04-04 12:39       ` Tanska, Kinga
2022-04-04 12:54         ` Paul Menzel
  -- strict thread matches above, loose matches on Subject: below --
2022-01-20 12:32 Kinga Tanska

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox