folkert/simh-testsetgenerator
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

SCP: Add'l memory sanitization fixes

Browse source 
Initialize de-dup'ed debug line buffer:

  realloc(NULL, size) == malloc(size), which is uninitialized space.
  This causes the Clang memory sanitizer to detect an attempt to read
  uninitialized memory when debug_line_buf and debug_line_buf_last are
  different lengths. While the uninitialized space may never actually be
  compared, the memory sanitizer emits a strong hint to not do stupid.

  The sanitizer trips in the i650 simulator on the first memcmp(),
  debug_line_buf has 108 characters, debug_line_buf_last has 56
  characters (uninitialized space follows the 56 characters, tripping
  the sanitizer.)

  - memset() debug_line_buf and debug_line_buf_last to zero so that
    memcmp() will always gracefully return non-zero if somehow memcmp()
    ends up going past the end of either buffer.  Should never happen in
    practice, but theory always gets mugged by reality.

  - Keep track of debug_line_buf_last's comparison length (i.e., up to
    the '\r') and only execute memcmp() when this length equals the
    current debug_line_buf comparison length (end - endprefix + 1).

  - Added a log deduplication test to "testlib" command to ensure that
    nothing broke as a result of this fix.

Network ACL check in sim_addr_acl_check:

  The memory sanitizer found an off-by-one bug in sim_addr_acl_check
  while executing "testlib". This makes CIDR network ACLs functional,
  e.g., "127.0.0.1/32" is interpreted properly and the associated
  "testlib" test passes.
This commit is contained in:
B. Scott Michel 2023-11-29 13:10:58 -08:00 • committed by Paul Koning
parent c077c22d24
commit f4c39a325c
2 changed files with 58 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
scp.c
View file

@ -335,6 +335,7 @@
#define SIM_DBG_DO 0x01000000 /* do activities */
#define SIM_DBG_SAVE 0x00800000 /* save activities */
#define SIM_DBG_RESTORE 0x00400000 /* restore activities */
#define SCP_LOG_TESTING 0x00200000 /* test_scp_debug_logging() debug */
static DEBTAB scp_debug[] = {
{"EVENT", SIM_DBG_EVENT, "Event Dispatch Activities"},
@ -347,6 +348,7 @@ static DEBTAB scp_debug[] = {
{"DO", SIM_DBG_DO, "Do Command/Expansion Activities"},
{"SAVE", SIM_DBG_SAVE, "Save Activities"},
{"RESTORE", SIM_DBG_RESTORE, "Restore Activities"},
{"LOG TEST", SCP_LOG_TESTING, "Log testing"},
{0}
};
@ -13560,6 +13562,7 @@ const char *debug_bstates = "01_^";
AIO_TLS char debug_line_prefix[256];
int32 debug_unterm = 0;
char *debug_line_buf_last = NULL;
size_t debug_line_buf_last_len = 0;
size_t debug_line_buf_last_endprefix_offset = 0;
AIO_TLS char debug_line_last_prefix[256];
char *debug_line_buf = NULL;
@ -13622,15 +13625,25 @@ if (sim_deb_switches & SWMASK ('F')) { /* filtering disabled? */
}
AIO_LOCK;
if (debug_line_offset + len + 1 > debug_line_bufsize) {
/* realloc(NULL, size) == malloc(size). Initialize the malloc()-ed space. Only
need to test debug_line_buf since SIMH allocates both buffers at the same
time. */
const int do_init = (NULL == debug_line_buf);
debug_line_bufsize += MAX(1024, debug_line_offset + len + 1);
debug_line_buf = (char *)realloc (debug_line_buf, debug_line_bufsize);
debug_line_buf_last = (char *)realloc (debug_line_buf_last, debug_line_bufsize);
if (do_init) {
memset(debug_line_buf, 0, debug_line_bufsize);
memset(debug_line_buf_last, 0, debug_line_bufsize);
}
}
memcpy (&debug_line_buf[debug_line_offset], buf, len);
debug_line_buf[debug_line_offset + len] = '\0';
debug_line_offset += len;
while ((eol = strchr (debug_line_buf, '\n')) || flush) {
char *endprefix = strstr (debug_line_buf, ")> ");
while (NULL != (eol = strchr (debug_line_buf, '\n')) || flush) {
const char *endprefix = strstr (debug_line_buf, ")> ");
size_t linesize = (eol - debug_line_buf) + 1;
if ((0 != memcmp ("DBG(", debug_line_buf, 4)) || (endprefix == NULL)) {
@ -13657,12 +13670,18 @@ while ((eol = strchr (debug_line_buf, '\n')) || flush) {
debug_line_buf_last_endprefix_offset = endprefix - debug_line_buf;
memcpy (debug_line_buf_last, debug_line_buf, linesize);
debug_line_buf_last[linesize] = '\0';
debug_line_buf_last_len = (NULL != eol) ? eol - endprefix + 1 : linesize;
debug_line_count = 1;
}
else {
if (0 == memcmp (&debug_line_buf[endprefix - debug_line_buf],
&debug_line_buf_last[debug_line_buf_last_endprefix_offset],
(eol - endprefix)+ 1)) {
const size_t compare_len = eol - endprefix + 1;
/* Ensure SIMH only executes memcmp() if the last message's comparison length
is the same as the current debug line's comparison length. */
if (debug_line_buf_last_len == compare_len &&
0 == memcmp (&debug_line_buf[endprefix - debug_line_buf],
&debug_line_buf_last[debug_line_buf_last_endprefix_offset],
compare_len)) {
++debug_line_count;
memcpy (debug_line_last_prefix, debug_line_buf, (endprefix - debug_line_buf) + 3);
debug_line_last_prefix[(endprefix - debug_line_buf) + 3] = '\0';
@ -13679,6 +13698,7 @@ while ((eol = strchr (debug_line_buf, '\n')) || flush) {
debug_line_buf_last_endprefix_offset = endprefix - debug_line_buf;
memcpy (debug_line_buf_last, debug_line_buf, linesize);
debug_line_buf_last[linesize] = '\0';
debug_line_buf_last_len = (NULL != eol) ? compare_len : linesize;
debug_line_count = 1;
}
}
@ -16442,6 +16462,36 @@ sim_set_deb_switches (start_deb_switches);
return r;
}
static t_stat test_scp_debug_logging()
{
uint32 saved_scp_dev_dbits = sim_scp_dev.dctrl;
FILE *saved_sim_deb = sim_deb;
sim_scp_dev.dctrl = SCP_LOG_TESTING;
sim_deb = stderr;
_sim_debug_device (SCP_LOG_TESTING, &sim_scp_dev, "Log message (repeats 3x)\n");
_sim_debug_device (SCP_LOG_TESTING, &sim_scp_dev, "Log message (repeats 3x)\n");
_sim_debug_device (SCP_LOG_TESTING, &sim_scp_dev, "Log message (repeats 3x)\n");
_sim_debug_device (SCP_LOG_TESTING, &sim_scp_dev, "Log message (repeats 3x)\n");
if (debug_line_count != 4)
return sim_messagef(SCPE_IERR, "expected debug_line_count == 4, actual %d\n", (int) debug_line_count);
_sim_debug_device (0xffffffff, &sim_scp_dev, "Different message.\n");
_sim_debug_flush ();
if (debug_line_count > 0)
return sim_messagef(SCPE_IERR, "expected debug_line_count == 0, actual %d\n", (int) debug_line_count);
sim_deb = saved_sim_deb;
sim_scp_dev.dctrl = saved_scp_dev_dbits;
sim_printf ("Log de-duplication successful.\n");
return SCPE_OK;
}
/*
* Compiled in unit tests for the various device oriented library
* modules: sim_card, sim_disk, sim_tape, sim_ether, sim_tmxr, etc.
@ -16486,6 +16536,8 @@ if ((strcmp (gbuf, "ALL") == 0) || (strcmp (gbuf, "SCP") == 0)) {
return sim_messagef (SCPE_IERR, "SCP argument parsing test failed\n");
if (test_scp_event_sequencing () != SCPE_OK)
return sim_messagef (SCPE_IERR, "SCP event sequencing test failed\n");
if (test_scp_debug_logging () != SCPE_OK)
return sim_messagef (SCPE_IERR, "SCP debug logging test failed\n");
}
for (i = 0; (dptr = sim_devices[i]) != NULL; i++) {
t_stat tstat = SCPE_OK;

2
sim_sock.c
View file

@ -721,7 +721,7 @@ if (c != NULL) {
if ((c - validate_addr) > sizeof (v_cpy) - 1)
return status;
memcpy (v_cpy, validate_addr, c - validate_addr); /* Copy everything before the / */
v_cpy[1 + c - validate_addr] = '\0'; /* NUL terminate the result */
v_cpy[c - validate_addr] = '\0'; /* NUL terminate the result */
validate_addr = v_cpy; /* Use the original string minus the prefix specifier */
}
if (p_getaddrinfo(validate_addr, NULL, NULL, &ai_validate))