From b1b4cbf583dfe54d344abbedb03c91084d228256 Mon Sep 17 00:00:00 2001 From: Toby Jaffey Date: Sat, 13 Dec 2025 18:46:28 +0000 Subject: [PATCH] Fix bug where memory was being used instead of extram when reading cstring from extram --- test/extram/rom/rom.c | 8 ++++ test/extram/shared.h | 2 + test/extram/test/tests.c | 93 ++++++++++++++++++++++++++++++++++++++-- uvm32/uvm32.c | 5 +-- 4 files changed, 101 insertions(+), 7 deletions(-) diff --git a/test/extram/rom/rom.c b/test/extram/rom/rom.c index 2bc3b0f..f94b0a7 100644 --- a/test/extram/rom/rom.c +++ b/test/extram/rom/rom.c @@ -61,6 +61,14 @@ void main(void) { p[5] = '\0'; println(p); // try to print from extram (terminated) } break; + case TEST11: { + // pass a string beyond end of ram + println((uint32_t)0xFFFFFFFF); + } break; + case TEST12: { + // pass a string beyond end of MMIO region + println((uint32_t)UVM32_EXTRAM_BASE + 8); // extram has been shrunk, this is now out of bounds, or no terminator found + } break; } } diff --git a/test/extram/shared.h b/test/extram/shared.h index f916cfe..a84d94f 100644 --- a/test/extram/shared.h +++ b/test/extram/shared.h @@ -12,5 +12,7 @@ enum { TEST8, TEST9, TEST10, + TEST11, + TEST12, }; diff --git a/test/extram/test/tests.c b/test/extram/test/tests.c index dac775a..58d1fce 100644 --- a/test/extram/test/tests.c +++ b/test/extram/test/tests.c @@ -217,13 +217,13 @@ void test_extram_buf_terminated(void) { // check for picktest syscall TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL); TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST); - uvm32_arg_setval(&vmst, &evt, RET, TEST9); + uvm32_arg_setval(&vmst, &evt, RET, TEST10); uvm32_run(&vmst, &evt, 100); TEST_ASSERT_EQUAL(true, uvm32_extramDirty(&vmst)); // check for printbuf of val TEST_ASSERT_EQUAL(UVM32_EVT_SYSCALL, evt.typ); - TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTBUF); + TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTLN); const char *str = uvm32_arg_getcstr(&vmst, &evt, ARG0); TEST_ASSERT_NOT_EQUAL(NULL, str); TEST_ASSERT_EQUAL(0, strcmp(str, "hello")); @@ -237,13 +237,13 @@ void test_extram_buf_terminated_rugpull(void) { // check for picktest syscall TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL); TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST); - uvm32_arg_setval(&vmst, &evt, RET, TEST9); + uvm32_arg_setval(&vmst, &evt, RET, TEST10); uvm32_run(&vmst, &evt, 100); TEST_ASSERT_EQUAL(true, uvm32_extramDirty(&vmst)); // check for printbuf of val TEST_ASSERT_EQUAL(UVM32_EVT_SYSCALL, evt.typ); - TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTBUF); + TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTLN); // remove extram uvm32_extram(&vmst, NULL, 0); @@ -256,4 +256,89 @@ void test_extram_buf_terminated_rugpull(void) { TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD); } +void test_extram_buf_terminated_beyond_mem_end(void) { + // run the vm + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + + // check for picktest syscall + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL); + TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST); + uvm32_arg_setval(&vmst, &evt, RET, TEST11); + + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + // check for printbuf of val + TEST_ASSERT_EQUAL(UVM32_EVT_SYSCALL, evt.typ); + TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTLN); + + // check that reading from non-existent extram gives empty string and puts into err state + const char *str = uvm32_arg_getcstr(&vmst, &evt, ARG0); + TEST_ASSERT_EQUAL(0, strlen(str)); + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_ERR); + TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD); +} + +void test_extram_buf_terminated_beyond_extram_end_oobstart(void) { + // run the vm + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + + // check for picktest syscall + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL); + TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST); + uvm32_arg_setval(&vmst, &evt, RET, TEST12); + + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + // check for printbuf of val + TEST_ASSERT_EQUAL(UVM32_EVT_SYSCALL, evt.typ); + TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTLN); + + // replace extram with a tiny one, so read is off the end + uint32_t r[1]; + r[0] = 0xFFFFFFFF; // non zero so string isn't terminated + uvm32_extram(&vmst, (uint8_t *)r, 4); + + // check that reading from non-existent extram gives empty string and puts into err state + const char *str = uvm32_arg_getcstr(&vmst, &evt, ARG0); + TEST_ASSERT_EQUAL(0, strlen(str)); + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_ERR); + TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD); +} + +void test_extram_buf_terminated_beyond_extram_end(void) { + // run the vm + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + + // check for picktest syscall + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_SYSCALL); + TEST_ASSERT_EQUAL(evt.data.syscall.code, SYSCALL_PICKTEST); + uvm32_arg_setval(&vmst, &evt, RET, TEST12); + + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(false, uvm32_extramDirty(&vmst)); + // check for printbuf of val + TEST_ASSERT_EQUAL(UVM32_EVT_SYSCALL, evt.typ); + TEST_ASSERT_EQUAL(evt.data.syscall.code, UVM32_SYSCALL_PRINTLN); + + // replace extram with a tiny one, so read is off the end + uint32_t r[3]; + // non-zero so string never terminates + memset(r, 0xAA, sizeof(r)); + uvm32_extram(&vmst, (uint8_t *)r, sizeof(r)); + + // string starts in valid extram, but no terminator will be found before hitting end of extram + + // check that reading from non-existent extram gives empty string and puts into err state + const char *str = uvm32_arg_getcstr(&vmst, &evt, ARG0); + TEST_ASSERT_EQUAL(0, strlen(str)); + uvm32_run(&vmst, &evt, 100); + TEST_ASSERT_EQUAL(evt.typ, UVM32_EVT_ERR); + TEST_ASSERT_EQUAL(evt.data.err.errcode, UVM32_ERR_MEM_RD); +} + diff --git a/uvm32/uvm32.c b/uvm32/uvm32.c index 37423b3..cfde7d4 100644 --- a/uvm32/uvm32.c +++ b/uvm32/uvm32.c @@ -142,8 +142,7 @@ bool get_safeptr_null_terminated(uvm32_state_t *vmst, uint32_t addr, uvm32_slice if (vmst->_extram == NULL) { return false; } else { - - uint32_t ptrstart = addr - UVM32_EXTRAM_BASE;; + uint32_t ptrstart = addr - UVM32_EXTRAM_BASE; uint32_t p = ptrstart; if (p >= vmst->_extramLen) { setStatusErr(vmst, UVM32_ERR_MEM_RD); @@ -151,7 +150,7 @@ bool get_safeptr_null_terminated(uvm32_state_t *vmst, uint32_t addr, uvm32_slice buf->len = 0; return false; } - while(vmst->_memory[p] != '\0') { + while(vmst->_extram[p] != '\0') { p++; if (p >= vmst->_extramLen) { setStatusErr(vmst, UVM32_ERR_MEM_RD);