#2248 UB in uimon_write_to_terminal()

[rice123]

UBSAN prints the following error when m 0 is executed in the monitor:

/vice/src/arch/gtk3/uimon.c:195:5: runtime error: null pointer passed as argument 1, which is declared to never be null

It happens when uimon_write_to_terminal() is called with the buffer being just "\n":

#0  uimon_write_to_terminal (t=t@entry=0x555558654e20 <fixed>, data=data@entry=0x5020002ddd90 "\n", length=length@entry=0) at /home/name/src/vice-emu-code/vice/src/arch/gtk3/uimon.c:169
#1  0x0000555556dacd44 in uimon_out (buffer=buffer@entry=0x5020002ddd90 "\n") at /home/name/src/vice-emu-code/vice/src/arch/gtk3/uimon.c:227
#2  0x0000555556a84f6c in mon_out_buffered (buffer=buffer@entry=0x5020002ddd90 "\n", mode=mode@entry=0, maxlen=1) at /home/name/src/vice-emu-code/vice/src/monitor/mon_util.c:223
#3  0x0000555556a85274 in mon_out (format=format@entry=0x5555570c5f20 "\n") at /home/name/src/vice-emu-code/vice/src/monitor/mon_util.c:258
#4  0x0000555556ad3cb5 in mon_memory_display (radix_type=<optimized out>, start_addr=<optimized out>, end_addr=<optimized out>, end_addr@entry=393216, format=format@entry=DF_PETSCII) at /home/name/src/vice-emu-code/vice/src/monitor/mon_memory.c:429
#5  0x0000555556aacd9d in yyparse () at /home/name/src/vice-emu-code/build/src/monitor/mon_parse.y:367
#6  0x0000555556abd88a in parse_and_execute_line (input=input@entry=0x5020002dd690 "m 0") at /home/name/src/vice-emu-code/build/src/monitor/mon_parse.y:1241
#7  0x0000555556a9959b in monitor_process (cmd=0x5020002dd690 "m 0") at /home/name/src/vice-emu-code/vice/src/monitor/monitor.c:3299
#8  monitor_startup (mem=mem@entry=e_default_space) at /home/name/src/vice-emu-code/vice/src/monitor/monitor.c:3415
#9  0x0000555556a9ae98 in monitor_trap (addr=<optimized out>, unused_data=<optimized out>) at /home/name/src/vice-emu-code/vice/src/monitor/monitor.c:3449
#10 0x00005555566155b5 in interrupt_do_trap (cs=0x7ffff2f97800, address=58831) at /home/name/src/vice-emu-code/vice/src/interrupt.c:357
#11 0x00005555566939d2 in maincpu_mainloop () at /home/name/src/vice-emu-code/vice/src/c64/../6510dtvcore.c:1763
#12 0x000055555662c579 in main_loop_forever () at /home/name/src/vice-emu-code/vice/src/main.c:583
#13 0x000055555662c5ae in vice_thread_main (unused=unused@entry=0x0) at /home/name/src/vice-emu-code/vice/src/main.c:632
#14 0x00007ffff785ea42 in asan_thread_start (arg=0x7fffccc04000) at ../../../../src/libsanitizer/asan/asan_interceptors.cpp:234
#15 0x00007ffff529cb84 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:447
#16 0x00007ffff5329d6c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

Since output_buffer_required_size > fixed.output_buffer_allocated_size is false when length == 0, no buffer is allocated and memcpy() is called with a null fixed.output_buffer. I think you can just bail out in such case:

diff --git a/vice/src/arch/gtk3/uimon.c b/vice/src/arch/gtk3/uimon.c
index 55f20c6380..ef028ce843 100644
--- a/vice/src/arch/gtk3/uimon.c
+++ b/vice/src/arch/gtk3/uimon.c
@@ -170,6 +170,10 @@ void uimon_write_to_terminal(struct console_private_s *t,
     size_t output_buffer_required_size;
     bool write_scheduled = false;


+    if (length == 0) {
+        return;
+    }
+
     pthread_mutex_lock(&fixed.lock);

     /*

btw, write_scheduled used to avoid adding unnecessary timeouts is set only when output_buffer_required_size > fixed.output_buffer_allocated_size. Maybe defining bool write_scheduled = !fixed.output_buffer; and removing write_scheduled = true; from the conditional branch can be more effective.

[gpz]

patched as proposed in r46203, thanks!