From ce378d3ec93d5edddb1d2081ce6ee9b07f6da287 Mon Sep 17 00:00:00 2001 From: Sebastien Godard Date: Sun, 20 Nov 2011 16:29:07 +0100 Subject: [PATCH] Fixed random crash with iostat when called with option -N [NOVELL Bug#729130]. Mail from Petr Uzel 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. --- CHANGES | 2 ++ ioconf.c | 7 +++++-- iostat.c | 2 +- pidstat.c | 3 +-- rndr_stats.c | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) 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 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; -- 2.40.0