blackmagic-debug / blackmagic

In application debugger for ARM Cortex microcontrollers.
GNU General Public License v3.0
3.13k stars 759 forks source link

Fix checks of `vasprintf` in gdb_packet.c #1834

Closed ALTracer closed 1 month ago

ALTracer commented 1 month ago

Detailed description

There are only two calls to the "allocating" function vasprintf in BMD, both in scope. I felt like adding *buf = NULL initialization for pointers on stack to be more defensive (during debug it was some 0x400 garbage). In the extremely pessimistic case of heap not working altogether, some of the first GDB RSP host packets trigger an exec_q_supported() reply with first vasprintf() which calls sbrk(0) internally, then sbrk(+72) and sbrk(+104); the second sbrk fails, failing malloc, returning -1, which indicates a) no [more] heap; b) buf is unpredictable (actually still 0x400). Every successful call to asprintf should be paired with eventual free, and every failed call should not be followed with free of that pointer*, I don't know how the compiler/optimizer/analyzer missed this (vasiprintf redefinition macro?) This code I'm touching is really old, including 02d37862 (good intent, meh implementation) and 018d9cce (correct, but what about va_end?); however, I'm somewhat sure that this way is better.

I don't know if there is a better way around this involving reporting this over RSP to gdb console (as O-packets) because interactive console is not open yet, so I added a couple DEBUG_ERRORs which do nothing (or log to ttyBmpTarg for log-enabled builds). Testing this in combination with #1830 I got this trace:

gdb_putpacket_f: vasprintf failed
gdb_voutf: vasprintf failed
Resetting TAP
Change state to Shift-DR
Scanning out ID codes
jtag_scan: Maximum chain length exceeded
calloc: failed in adiv5_swd_scan
gdb_voutf: vasprintf failed
calloc: failed in adiv5_swd_scan

resulting from opening target extended-remote /dev/ttyBmpGdb, commanding monitor auto_scan, monitor swdp_scan. Both new messages are present. In the "rare" case of heap exhaustion on stlink/swlink this should no longer explode, if it did, I think. Tested on stlink/bluepill platform. GDB did complain, but then allowed me issuing most monitor commands, with firmware responding with Ok Acks but no formatted O-packets, only static/sprintf ones (Debug Enabled, Scan Failed etc.) No JTAG or SWD chains could be scanned because DP requires a (small) malloc.

Your checklist for this pull request

Closing issues

Helps #1830 run (technically) with zero heap usage.