Trivial crash when attempting to close stdin/stdout

[gwangmu]

Description

We discovered a trivial crash case when MicroPython attempted to close stdin or stdout. Interestingly, it didn't crash with stderr. The reference behavior from CPython was simply allowing such close() operations.

Proof of Concept

File: poc.py

import sys
v1 = sys.stdout   # same crash happens with sys.stdin
v1.close()

Execution:

$ # build unix port with ASAN, at the root source code directory.
$ export CC=clang
$ export CXX=clang++
$ export CFLAGS="-fsanitize=address -fno-omit-frame-pointer"
$ export CXXFLAGS=$CFLAGS
$ export LDFLAGS=$CFLAGS
$ export DEBUG=1
$ make -C mpy-cross -j
$ make -C ports/unix -j all lib
$
$ # run a poc.
$ export ASAN_OPTIONS="detect_leaks=0"
$ ./ports/unix/build-standard/micropython poc.py

Stack trace:

=================================================================
==373731==ERROR: AddressSanitizer: SEGV on unknown address 0x562ee4d95bc8 (pc 0x562ee4c76afe bp 0x7ffd6a25c260 sp 0x7ffd6a25c240 T0)
==373731==The signal is caused by a WRITE memory access.
    #0 0x562ee4c76afe in vfs_posix_file_ioctl ../../extmod/vfs_posix_file.c:192
    #1 0x562ee4c33e78 in mp_stream_close ../../py/stream.c:433
    #2 0x562ee4c12691 in fun_builtin_1_call ../../py/objfun.c:68
    #3 0x562ee4bfd4ec in mp_call_function_n_kw ../../py/runtime.c:712
    #4 0x562ee4bfd96b in mp_call_method_n_kw ../../py/runtime.c:728
    #5 0x562ee4c4375d in mp_execute_bytecode ../../py/vm.c:1042
    #6 0x562ee4c12e46 in fun_bc_call ../../py/objfun.c:273
    #7 0x562ee4bfd4ec in mp_call_function_n_kw ../../py/runtime.c:712
    #8 0x562ee4bfd546 in mp_call_function_0 ../../py/runtime.c:686
    #9 0x562ee4d0bcfd in execute_from_lexer /workspaces/Latest/micropython/ports/unix/main.c:160
    #10 0x562ee4d0bdc2 in do_file /workspaces/Latest/micropython/ports/unix/main.c:309
    #11 0x562ee4d0cded in main_ /workspaces/Latest/micropython/ports/unix/main.c:721
    #12 0x562ee4d0d21a in main /workspaces/Latest/micropython/ports/unix/main.c:476
    #13 0x7fa7ffd99d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #14 0x7fa7ffd99e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #15 0x562ee4bbc4c4 in _start (/workspaces/Latest/micropython/ports/unix/build-standard/micropython+0x854c4)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV ../../extmod/vfs_posix_file.c:192 in vfs_posix_file_ioctl
==373731==ABORTING

Environment

Ubuntu 20.04
Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz
Memory: 64 GB

Affected Version

v1.20.0 (commit a3862e7, latest as of 2023-09-26)
v1.20.0 (commit 813d559, 2023-06-19)
Discovered in the UNIX port version.

[stinos]

Strange, this seems to segfault on the o->fd = -1 statement and removing that line fixes it, but I don't see anything in gdb causing that, as in: memory from o seems untouched after call to close. Don't have much experience with it though.

[gwangmu]

I suspect if that was a sort of use-after-free crash because ASAN sanitizes freed memories (so as to detect uses after frees) but GDB doesn't do it to my knowledge. If o was freed by libc but still in a mapped memory page, GDB would assume the uses on it as a valid memory access. But that being said, I don't know why it was untouched after supposedly assigning -1 to it.

[stinos]

This happened without ASAN, with a normal build of current master, also when not running under GDB and also with or without passing DEBUG=1.

Checked again in disassembly view and the instruction for which the segfault occurs is effectively the assiging to fd (well, or VSCode/gdb are showing me things which aren't real but I doubt it):

mov DWORDT PTR [rbp+0x8],0xffffffff

and at that point rbp has address of o and 8 is the offset of fd, and the call to close did (as far as VSCode shows) not change anything in o.

So this leads me to think the issue is that this piece of memory is read-only, after all mp_sys_stdout_obj is declared const (note: for me closing stderr does have the same problem). And indeed removing the const makes the segfault go away. So seems a 'classic' case of undefined behavior: casting const away is not good..

[dpgeorge]

Fixed by 3659139