From: Sebastien Godard <sysstat@orange.fr>
Date: Sun, 20 Nov 2011 15:29:07 +0000 (+0100)
Subject: Fixed random crash with iostat when called with
X-Git-Tag: v10.0.3~2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ce378d3ec93d5edddb1d2081ce6ee9b07f6da287;p=sysstat

Fixed random crash with iostat when called with
option -N [NOVELL Bug#729130].

Mail from Petr Uzel <petr.uzel@suse.cz> 11/13/2011 11:45 AM:

> > On 11/09/2011 01:34 PM, Petr Uzel wrote:
>> > >attached patch fixes
>> > >https://bugzilla.novell.com/show_bug.cgi?id=729130
>> > >
Hi Sebastien,

As far as I understand (please correct me if I'm wrong), sysstat is
hitting unspecified behavior, which might explain why you can not
reproduce the bug.

Check out transfrom_devmapname() function:

	while ((dp = readdir(dm_dir)) != NULL) {
		/* For each file in DEVMAP_DIR */

		snprintf(filen, MAX_FILE_LEN, "%s/%s", DEVMAP_DIR, dp->d_name);
		filen[MAX_FILE_LEN - 1] = '\0';

		if (stat(filen, &aux) == 0) {
			/* Get its minor and major numbers */

			dm_major = major(aux.st_rdev);
			dm_minor = minor(aux.st_rdev);

			if ((dm_minor == minor) && (dm_major == major)) {
[********>] 			dm_name = dp->d_name;
				break;
			}
		}
	}
	closedir(dm_dir);

dm_name points to the memory returned by readdir(), but from 'man
readdir', this memory is not guaranteed to be valid after next call
to readdir or after closedir().

man readdir:

       ....
       The data returned by readdir() may be overwritten by subsequent
       calls to readdir() for the same directory stream.
       ....

man closedir:

       ....
       The  closedir()  function closes the directory stream
       associated with dirp.  A successful call to closedir() also
       closes the underlying file descriptor associated with dirp.
       The directory stream descriptor dirp is not available after
       this call.
       ....

[It is not very clear to me if this also invalidates the dirent
structure]

So the solution is to strncpy the memory before it gets invalidated by
next readdir() or closedir().

Unrelated to this bug:
Could you please add something like make install_nostrip to the
Makefile? By default, the buildsystem uses LDFLAGS = -s, which always
strips the resulting binary. In openSUSE, we have to patch this,
because we need to strip the binaries on our own (to create
sysstat-debug{source,info} packages).

Thanks for your support,

	Petr.
---

diff --git a/CHANGES b/CHANGES
index 39f9e4c..3cdf038 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,8 @@ xxxx/xx/xx: Version 10.0.3 - Sebastien Godard (sysstat <at> orange.fr)
 	* [Jürgen Heinemann]: Fixed a bug in sadf XML output.
 	* [Jürgen Heinemann]: Fixed several bugs in DTD and XSD
 	  documents.
+	* [Petr Uzel]: Fixed random crash with iostat when called with
+	  option -N [NOVELL Bug#729130].
 	* sadf manual page updated.
 	* CREDITS file updated.
 
diff --git a/ioconf.c b/ioconf.c
index b388683..c3929ca 100644
--- a/ioconf.c
+++ b/ioconf.c
@@ -483,6 +483,7 @@ char *transform_devmapname(unsigned int major, unsigned int minor)
 	struct dirent *dp;
 	char filen[MAX_FILE_LEN];
 	char *dm_name = NULL;
+	static char name[MAX_NAME_LEN];
 	struct stat aux;
 	unsigned int dm_major, dm_minor;
 
@@ -502,9 +503,11 @@ char *transform_devmapname(unsigned int major, unsigned int minor)
 
 			dm_major = major(aux.st_rdev);
 			dm_minor = minor(aux.st_rdev);
-	
+
 			if ((dm_minor == minor) && (dm_major == major)) {
-				dm_name = dp->d_name;
+				strncpy(name, dp->d_name, MAX_NAME_LEN);
+				name[MAX_NAME_LEN - 1] = '\0';
+				dm_name = name;
 				break;
 			}
 		}
diff --git a/iostat.c b/iostat.c
index d2f0f41..95d5b4d 100644
--- a/iostat.c
+++ b/iostat.c
@@ -686,7 +686,7 @@ void read_diskstats_stat(int curr)
 			 */
 			dm_name = transform_devmapname(major, minor);
 			if (dm_name) {
-				strcpy(dev_name, dm_name);
+				strncpy(dev_name, dm_name, MAX_NAME_LEN);
 			}
 		}
 
diff --git a/pidstat.c b/pidstat.c
index 36ef142..1e03c7f 100644
--- a/pidstat.c
+++ b/pidstat.c
@@ -1217,8 +1217,7 @@ int write_pid_task_all_stats(int prev, int curr, int dis,
  ***************************************************************************
  */
 int write_pid_child_all_stats(int prev, int curr, int dis,
-			     unsigned long long itv)
-			
+			      unsigned long long itv)
 {
 	struct pid_stats *pstc, *pstp;
 	unsigned int p;
diff --git a/rndr_stats.c b/rndr_stats.c
index 293bf62..d280bf3 100644
--- a/rndr_stats.c
+++ b/rndr_stats.c
@@ -2473,7 +2473,7 @@ __print_funct_t render_pwr_fan_stats(struct activity *a, int isdb, char *pre,
  ***************************************************************************
  */
 __print_funct_t render_pwr_temp_stats(struct activity *a, int isdb, char *pre,
-                                        int curr, unsigned long long itv)
+				      int curr, unsigned long long itv)
 {
 	int i;
 	struct stats_pwr_temp *spc;