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 39f9e4cc0960410e7a25b40a1659d89465134c61..3cdf038a329a82226cfdf4f653125c2b7fe0159e 100644 (file)
--- 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 b38868374cfa854c7427282f4df43ceea3b2b968..c3929ca1e5ebde250cc26eea72ad8bd41fcb5e36 100644 (file)
--- 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 d2f0f41b2405822aa5d6d7ba5bdbae7adcc422a0..95d5b4d7d2e5cc74c0e5e397d359ce29e8833f0a 100644 (file)
--- 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 36ef142a272bf2f6cfe7824a3b9a4fcb98b5cb67..1e03c7f9ca6b0e70e78bf6042fec343f844ac191 100644 (file)
--- 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 293bf6255fa47278f37f06d02d063a6502d33d71..d280bf3168ff33380e622f021a231dce9296fb4a 100644 (file)
--- 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;