Fix potential overflow with some arrays at page-isolation logic

Overflows may happen in the `threshold_string` and `cycle_string` arrays.

If the PAGE_CE_THRESHOLD value in page isolation is set to 50 bits,
there is a risk of array overflow. Because sprintf is an insecure
function, use snprintf instead.

An error is reported when the AddressSanitizer is used.

rasdaemon: Improper PAGE_CE_ACTION, set to default soft
rasdaemon: Page offline choice on Corrected Errors is soft
=================================================================
==221920==ERROR: AddressSanitizer: stack-buffer-overflow on address 0xffffdd91d932 at pc 0xffffa24071c4 bp 0xffffdd91d720 sp 0xffffdd91ced8
WRITE of size 55 at 0xffffdd91d932 thread T0
    #0 0xffffa24071c0 in vsprintf (/usr/lib64/libasan.so.6+0x5c1c0)
    #1 0xffffa24073cc in sprintf (/usr/lib64/libasan.so.6+0x5c3cc)
    #2 0x459558 in parse_env_string /home/rasdaemon/ras-page-isolation.c:185
    #3 0x4596f4 in page_isolation_init /home/rasdaemon/ras-page-isolation.c:202
    #4 0x459934 in ras_page_account_init /home/rasdaemon/ras-page-isolation.c:211
    #5 0x40f700 in handle_ras_events /home/rasdaemon/ras-events.c:902
    #6 0x405b8c in main /home/rasdaemon/rasdaemon.c:211
    #7 0xffffa20b6f38 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0xffffa20b7004 in __libc_start_main_impl ../csu/libc-start.c:409
    #9 0x4038ec in _start (/home/rasdaemon/rasdaemon+0x4038ec)

Address 0xffffdd91d932 is located in stack of thread T0 at offset 82 in frame
    #0 0x459574 in page_isolation_init /home/rasdaemon/ras-page-isolation.c:190

  This frame has 2 object(s):
    [32, 82) 'threshold_string' (line 191)
    [128, 178) 'cycle_string' (line 192) <== Memory access at offset 82 partially underflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (/usr/lib64/libasan.so.6+0x5c1c0) in vsprintf
Shadow bytes around the buggy address:
  0x200ffbb23ad0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200ffbb23ae0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200ffbb23af0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200ffbb23b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200ffbb23b10: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
=>0x200ffbb23b20: 00 00 00 00 00 00[02]f2 f2 f2 f2 f2 00 00 00 00
  0x200ffbb23b30: 00 00 02 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x200ffbb23b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x200ffbb23b50: f1 f1 f1 f1 f1 f1 04 f2 00 00 f2 f2 00 00 00 00
  0x200ffbb23b60: 00 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 f2
  0x200ffbb23b70: f2 f2 f2 f2 00 00 00 00 00 00 00 00 f2 f2 f2 f2
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==221920==ABORTING

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

diff --git a/ras-page-isolation.c b/ras-page-isolation.c
index fd7bd704dc8e886d4260989e458ef8584714c77c..caa8c31836617fa70795f1e097160b04cc9c7731 100644 (file)
--- a/ras-page-isolation.c
+++ b/ras-page-isolation.c
@@ -171,18 +171,18 @@ parse:
        config->unit = no_unit ? config->unit : "";
 }
 
-static void parse_env_string(struct isolation *config, char *str)
+static void parse_env_string(struct isolation *config, char *str, unsigned int size)
 {
        int i;
 
        if (config->overflow) {
                /* when overflow, use basic unit */
                for (i = 0; config->units[i].name; i++) ;
-               sprintf(str, "%lu%s", config->val, config->units[i-1].name);
+               snprintf(str, size, "%lu%s", config->val, config->units[i-1].name);
                log(TERM, LOG_INFO, "%s is set overflow(%s), truncate it\n",
                                config->name, config->env);
        } else {
-               sprintf(str, "%s%s", config->env, config->unit);
+               snprintf(str, size, "%s%s", config->env, config->unit);
        }
 }
 
@@ -199,8 +199,8 @@ static void page_isolation_init(void)
 
        parse_isolation_env(&threshold);
        parse_isolation_env(&cycle);
-       parse_env_string(&threshold, threshold_string);
-       parse_env_string(&cycle, cycle_string);
+       parse_env_string(&threshold, threshold_string, sizeof(threshold_string));
+       parse_env_string(&cycle, cycle_string, sizeof(cycle_string));
        log(TERM, LOG_INFO, "Threshold of memory Corrected Errors is %s / %s\n",
                        threshold_string, cycle_string);
 }