From: zhuofeng <zhuofeng2@huawei.com>
Date: Thu, 7 Dec 2023 02:26:56 +0000 (+0800)
Subject: Fix potential overflow with some arrays at page-isolation logic
X-Git-Tag: v0.8.1~51
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=0e823890cafdb7220bc916f77b21ca2bdf6cdadc;p=users%2Fmchehab%2Frasdaemon.git

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 fd7bd70..caa8c31 100644
--- 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);
 }