From f202a317994f563a25a8c64a38ba4445a1d4606b Mon Sep 17 00:00:00 2001 From: "Zachary P. Landau" Date: Sun, 16 Feb 2020 12:33:30 -0800 Subject: [PATCH] Fix crash when system reports huge load averages The load average reporting functions in showsys.c use static buffer sizes. When the load averages on a machine are very large, this causes the writes to extend past the buffer. With this commit, if a number is too large then we just show '>NNNNNN'. I'm not sure if this is the best choice, so I'm open to other ideas. This is what the output looks like when we exceed the maximums: CPL | avg1 >999999 | avg5 >999999 | avg15 >99999 | csw 103117e3 | intr 88296e3 | Note that this was triggered from a kernel that is reporting clearly inaccurate numbers: $ cat /proc/loadavg 1.25 2.40 368567045.47 1/589 53576 But regardless, crashing is no fun. For future reference, I narrowed down the issue by building with -fsanitize=address. For example: ==55396==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55c527d6f14f at pc 0x7f4729942df9 bp 0x7ffe1fd30710 sp 0x7ffe1fd2fea0 WRITE of size 10 at 0x55c527d6f14f thread T0 #0 0x7f4729942df8 in __interceptor_vsprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1627 #1 0x7f47299432cf in __interceptor_sprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1670 #2 0x55c527d1c167 in sysprt_CPLAVG15 (/home/kapheine/projects/atop/atop+0x74167) #3 0x55c527d2087a in showsysline (/home/kapheine/projects/atop/atop+0x7887a) #4 0x55c527d1471f in prisyst (/home/kapheine/projects/atop/atop+0x6c71f) #5 0x55c527d090ff in generic_samp (/home/kapheine/projects/atop/atop+0x610ff) #6 0x55c527ce17c9 in main (/home/kapheine/projects/atop/atop+0x397c9) #7 0x7f472952a022 in __libc_start_main (/usr/lib/libc.so.6+0x27022) #8 0x55c527ce266d in _start (/home/kapheine/projects/atop/atop+0x3a66d) 0x55c527d6f14f is located 49 bytes to the left of global variable 'buf' defined in 'showsys.c:981:54' (0x55c527d6f180) of size 15 0x55c527d6f14f is located 0 bytes to the right of global variable 'buf' defined in 'showsys.c:1000:54' (0x55c527d6f140) of size 15 --- showsys.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/showsys.c b/showsys.c index 494cefab..933a0f7b 100644 --- a/showsys.c +++ b/showsys.c @@ -959,9 +959,13 @@ char * sysprt_CPLAVG1(void *p, void *notused, int badness, int *color) { struct sstat *sstat=p; - static char buf[15]="avg1 "; + static char buf[17]="avg1 "; - if (sstat->cpu.lavg1 > 999.0) + if (sstat->cpu.lavg1 > 999999.0) + { + sprintf(buf+5, ">999999"); + } + else if (sstat->cpu.lavg1 > 999.0) { sprintf(buf+5, "%7.0f", sstat->cpu.lavg1); } @@ -980,7 +984,11 @@ sysprt_CPLAVG5(void *p, void *notused, int badness, int *color) struct sstat *sstat=p; static char buf[15]="avg5 "; - if (sstat->cpu.lavg5 > 999.0) + if (sstat->cpu.lavg5 > 999999.0) + { + sprintf(buf+5, ">999999"); + } + else if (sstat->cpu.lavg5 > 999.0) { sprintf(buf+5, "%7.0f", sstat->cpu.lavg5); } @@ -1002,7 +1010,11 @@ sysprt_CPLAVG15(void *p, void *notused, int badness, int *color) if (sstat->cpu.lavg15 > (2 * sstat->cpu.nrcpu) ) *color = COLORALMOST; - if (sstat->cpu.lavg15 > 999.0) + if (sstat->cpu.lavg15 > 99999.0) + { + sprintf(buf+6, ">99999"); + } + else if (sstat->cpu.lavg15 > 999.0) { sprintf(buf+6, "%6.0f", sstat->cpu.lavg15); }