diff mbox series

[3/9] e2fsck: avoid mallinfo() if over 2GB allocated

Message ID 1581037786-62789-3-git-send-email-adilger@whamcloud.com
State Accepted
Headers show
Series [1/9] e2fsck: fix e2fsck_allocate_memory() overflow | expand

Commit Message

Andreas Dilger Feb. 7, 2020, 1:09 a.m. UTC 
Don't use mallinfo() for determining the amount of memory used if it
is over 2GB.  Otherwise, the signed ints used by this interface can
can overflow and return garbage values.  This makes the actual amount
of memory used by e2fsck misleading and hard to determine.

Instead, use brk() to get the total amount of memory allocated, and print
this if the more detailed mallinfo() information is not suitable for use.

There does not appear to be a mallinfo64() variant of this function.
There does appear to be an abomination named malloc_info() that writes
XML-formatted malloc stats to a FILE stream that would need to be read
and parsed in order to get these stats, but that doesn't seem worthwhile.

Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Shilong Wang <wshilong@ddn.com>
Lustre-bug-id: https://jira.whamcloud.com/browse/LU-13197
---
 e2fsck/scantest.c |  4 ++--
 e2fsck/util.c     | 26 +++++++++++++-------------
 2 files changed, 15 insertions(+), 15 deletions(-)

Comments

Theodore Ts'o Feb. 29, 2020, 11:28 p.m. UTC | #1 
On Thu, Feb 06, 2020 at 06:09:40PM -0700, Andreas Dilger wrote:
> Don't use mallinfo() for determining the amount of memory used if it
> is over 2GB.  Otherwise, the signed ints used by this interface can
> can overflow and return garbage values.  This makes the actual amount
> of memory used by e2fsck misleading and hard to determine.
> 
> Instead, use brk() to get the total amount of memory allocated, and print
> this if the more detailed mallinfo() information is not suitable for use.
> 
> There does not appear to be a mallinfo64() variant of this function.
> There does appear to be an abomination named malloc_info() that writes
> XML-formatted malloc stats to a FILE stream that would need to be read
> and parsed in order to get these stats, but that doesn't seem worthwhile.
> 
> Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
> Reviewed-by: Shilong Wang <wshilong@ddn.com>
> Lustre-bug-id: https://jira.whamcloud.com/browse/LU-13197

Applied, thanks.

					- Ted
diff mbox series

Patch

diff --git a/e2fsck/scantest.c b/e2fsck/scantest.c
index 6131141..ed3595f 100644
--- a/e2fsck/scantest.c
+++ b/e2fsck/scantest.c
@@ -76,8 +76,8 @@  static void print_resource_track(struct resource_track *track)
 	gettimeofday(&time_end, 0);
 	getrusage(RUSAGE_SELF, &r);
 
-	printf(_("Memory used: %d, elapsed time: %6.3f/%6.3f/%6.3f\n"),
-	       (int) (((char *) sbrk(0)) - ((char *) track->brk_start)),
+	printf(_("Memory used: %lu, elapsed time: %6.3f/%6.3f/%6.3f\n"),
+	       (unsigned long)((char *)sbrk(0) - (char *)track->brk_start),
 	       timeval_subtract(&time_end, &track->time_start),
 	       timeval_subtract(&r.ru_utime, &track->user_start),
 	       timeval_subtract(&r.ru_stime, &track->system_start));
diff --git a/e2fsck/util.c b/e2fsck/util.c
index 300993d..e3d92b3 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -421,9 +421,6 @@  void print_resource_track(e2fsck_t ctx, const char *desc,
 #ifdef HAVE_GETRUSAGE
 	struct rusage r;
 #endif
-#ifdef HAVE_MALLINFO
-	struct mallinfo	malloc_info;
-#endif
 	struct timeval time_end;
 
 	if ((desc && !(ctx->options & E2F_OPT_TIME2)) ||
@@ -436,18 +433,21 @@  void print_resource_track(e2fsck_t ctx, const char *desc,
 	if (desc)
 		log_out(ctx, "%s: ", desc);
 
+#define kbytes(x)	(((unsigned long long)(x) + 1023) / 1024)
 #ifdef HAVE_MALLINFO
-#define kbytes(x)	(((unsigned long)(x) + 1023) / 1024)
-
-	malloc_info = mallinfo();
-	log_out(ctx, _("Memory used: %luk/%luk (%luk/%luk), "),
-		kbytes(malloc_info.arena), kbytes(malloc_info.hblkhd),
-		kbytes(malloc_info.uordblks), kbytes(malloc_info.fordblks));
-#else
-	log_out(ctx, _("Memory used: %lu, "),
-		(unsigned long) (((char *) sbrk(0)) -
-				 ((char *) track->brk_start)));
+	/* don't use mallinfo() if over 2GB used, since it returns "int" */
+	if ((char *)sbrk(0) - (char *)track->brk_start < 2ULL << 30) {
+		struct mallinfo	malloc_info = mallinfo();
+
+		log_out(ctx, _("Memory used: %lluk/%lluk (%lluk/%lluk), "),
+			kbytes(malloc_info.arena), kbytes(malloc_info.hblkhd),
+			kbytes(malloc_info.uordblks),
+			kbytes(malloc_info.fordblks));
+	} else
 #endif
+	log_out(ctx, _("Memory used: %lluk, "),
+		kbytes(((char *)sbrk(0)) - ((char *)track->brk_start)));
+
 #ifdef HAVE_GETRUSAGE
 	getrusage(RUSAGE_SELF, &r);