esp8266: strncpy() segfaults on empty password in webrepl's 'port_config.py'

[aexaey]

Initial setup of webrepl would happily accept empty password (which is fine, and by design, I suppose?), but then _webrepl.password() would segfault on strncpy().

Here is instrumentation added:

--- a/extmod/modwebrepl.c
+++ b/extmod/modwebrepl.c
@@ -285,8 +285,12 @@ STATIC mp_uint_t webrepl_write(mp_obj_t self_in, const void *buf, mp_uint_t size
 }

 STATIC mp_obj_t webrepl_set_password(mp_obj_t passwd_in) {
+    printf("in webrepl_set_password()\n");
     const char *passwd = mp_obj_str_get_str(passwd_in);
+    printf("*passwd is '%s'\n", passwd);
+    printf("*passwd is at 0x%08x\n", (unsigned int)(void *)passwd);
     strncpy(webrepl_passwd, passwd, sizeof(webrepl_passwd));
+    printf("strncpy() returned\n");
     return mp_const_none;
 }

Here is log of a failing test on the commit 1f0dfe3 + instrumentation above:

>>> with open('port_config.py', 'w') as f: print(f.write("WEBREPL_PASS = ''"))
...
17
>>> import webrepl
>>> webrepl.start()
in webrepl_set_password()
*passwd is ''
*passwd is at 0x4026ab11
Fatal exception 3(LoadStoreErrorCause):
epc1=0x4000c068, epc2=0x00000000, epc3=0x00000000, excvaddr=0x4026ab11, depc=0x00000000

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

load 0x40100000, len 30548, room 16
...

Same with non-empty password succeeds as expected:

>>> with open('port_config.py', 'w') as f: print(f.write("WEBREPL_PASS = '123'"))
...
20
>>> import webrepl
>>> webrepl.start()
in webrepl_set_password()
*passwd is '123'
*passwd is at 0x3fff0175
strncpy() returned
WebREPL daemon started on ws://192.168.4.1:8266
Started webrepl in normal mode
>>>

Looking at the *passwd is at line above, it appears that normal password is stored at 0x3fff0175, i.e. "ETS system data RAM" section, while empty password - under a pointer to 0x4026ab11, i.e. "SPI flash" memory range, according to [1]. Exact same address is to be seen in "excvaddr" which I would guess implies that direct memory access to SPI could sometimes be denied - artifact of ESP-specific paging/caching?

Actual data stored on 0x4026ab11 appears to be '\0', i.e. empty C string, which sort of makes sense:

$ xtensa-lx106-elf-objdump -d esp8266/build/firmware.elf | grep 4026ab[01]
4026ab0c:       0000 1505 0000 00ea 4b08 7965 7245 6f72     .........KeyErro
4026ab1c:       0072 006b 5f0c 715f 6175 6e6c 6d61 5f65     r.k..__qualname_

$ cat esp8266/build/firmware.map | grep 4026ab
 .rodata        0x000000004026ab0a       0x53 build/py/qstr.o
 *fill*         0x000000004026ab5d        0x3
                0x000000004026ab60      0x908 build/py/qstr.o
                0x000000004026ab60                mp_qstr_const_pool

Strangely enough, accessing very same data from python code works just fine - maybe because interpreter has a special case for "empty string constant" that it compared by address without actually accessing it?

>>> import _webrepl
>>> with open('port_config.py', 'w') as f: print(f.write("WEBREPL_PASS = ''"))
...
17
>>> import port_config
>>> port_config.WEBREPL_PASS
''
>>> _webrepl.password(port_config.WEBREPL_PASS)
in webrepl_set_password()
*passwd is ''
*passwd is at 0x4026ab11
Fatal exception 3(LoadStoreErrorCause):
epc1=0x4000c068, epc2=0x00000000, epc3=0x00000000, excvaddr=0x4026ab11, depc=0x00000000

 ets Jan  8 2013,rst cause:2, boot mode:(3,6)

[1] https://github.com/esp8266/esp8266-wiki/wiki/Memory-Map

[aexaey]

I think I know what is going on here.

strncpy() is in the "Internal ROM" according to the ESP memory map:

$ cat esp8266/build/firmware.map | grep strncpy
                0x0000000040002a98                PROVIDE (ets_strncpy, 0x40002a98)
                0x000000004000c0a0                PROVIDE (strncpy, 0x4000c0a0)

...and ROM can't access SPI Flash on ESP (not at usual 0x40200000 at least) - something to do with the "SPI Flash mapping hardware" not actually being hardware, but rather it is some clever routine in ROM that sits on segfault interrupt and does the actual mapping, complicated with some quirks like that ROM can't catch segfault from itself, or something... Either way, this seem to be a known problem - see Arduino/esp8266's "pgmspace" module:
https://github.com/esp8266/Arduino/blob/master/cores/esp8266/pgmspace.cpp

Borrowing strncpy_P from there:

--- a/extmod/modwebrepl.c
+++ b/extmod/modwebrepl.c
@@ -284,11 +284,28 @@ STATIC mp_uint_t webrepl_write(mp_obj_t self_in, const void *buf, mp_uint_t size
     return stream_p->write(self->sock, buf, size, errcode);
 }

+char* ICACHE_FLASH_ATTR strncpy_P(char* dest, const char* src, size_t size) {
+    const char* read = src;
+    char* write = dest;
+    char ch = '.';
+    while (size > 0 && ch != '\0') {
+        ch = *read++;
+        *write++ = ch;
+        size--;
+    }
+    return dest;
+}
+
 STATIC mp_obj_t webrepl_set_password(mp_obj_t passwd_in) {
+    printf("in webrepl_set_password()\n");
     const char *passwd = mp_obj_str_get_str(passwd_in);
+    printf("*passwd is '%s'\n", passwd);
+    printf("*passwd is at 0x%08x\n", (unsigned int)(void *)passwd);
-    strncpy(webrepl_passwd, passwd, sizeof(webrepl_passwd));
+    strncpy_P(webrepl_passwd, passwd, sizeof(webrepl_passwd));
+    printf("strncpy_P() returned\n");
     return mp_const_none;
 }
+

...seem to fix this:

>>> import _webrepl
>>> _webrepl.password('')
in webrepl_set_password()
*passwd is ''
*passwd is at 0x4026ab39
strncpy_P() returned
>>>

[pfalcon]

and by design, I suppose?

No, it's initial implementation which is gets the main flow right, but leaves out gazillion of corner cases.

something to do with the "SPI Flash mapping hardware" not actually being hardware, but rather it is some clever

No, it's real hardware and it has real and pretty simple constraints - on access alignment and size. Good reference on background of it I can give is the essay which was published as a part of Kickstarter campaign: https://www.kickstarter.com/projects/214379695/micropython-on-the-esp8266-beautifully-easy-iot/posts/1501224

Overall, great research and debugging, I myself was confused how this may happen for this case, your dumps made it all clear, excellent work!

@dpgeorge : So, we were pleasantly surprised that -mforce-l32 works even when you wouldn't think it does, and it's 2nd time I got bitten by it "unexpectedly" not working, dealing with this webrepl password prompt.

First time, I passed stuff from .rodata to lwip, it's not built with -mforce-l32, so kaboom. Here, it's even more confusing. First thought about strncpy() in libc which is again not built with -mforce-l32. But we build with -nostdlib. And then - destination buffer is in dRAM, source string is in dRAM, what can go wrong? Well, ROMmed qstr's! And another unclear q how BootROM funcs are involved? Well, libc's not used, no strncpy() impl in uPy codebase, then lowest-prio PROVIDE in .ld actually kicks in.

So, I guess the least resistance workaround is do what @aexaey suggests and use own strncpy impl. My initial inclination was to name it safe_strncpy() and add to esp_mphal.c, but then the issue may happen again and again. So, maybe add to lib/libc/string0.c? Well, the easiest way would be to replaces strncpy() with memcpy()...

[pfalcon]

Ok, we have enough stuff in string0.c, so no big deal add another func to be --gc-sections'ed if not needed. Done.

[dpgeorge]

@pfalcon I would suggest not to use strncpy at all (it's not used in any of our code at all, except now in webrepl...). It's too error prone, and the implementation you provided is wrong (no null should be added if there is no room at the end). So I'd recode the webrepl_set_password function to use memcpy and then explicitly add a null terminator.

[pfalcon]

no null should be added if there is no room at the end

LOL, live and learn wonderful C standards we have! Will revert and redo.