On Thu, May 16, 2024 at 09:32:36AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > - the opposite approach might be: stop using any allocating functions > > in the trace2 code. There's a certain simplicity there, even for > > non-signal functions, that we know we're just touching a few > > fixed-size buffers, and you can never create a weird DoS by tweaking > > the tracing code. But it would mean rewriting a lot of it (including > > json formatting stuff) without many of our usual strbuf niceties. > > > > This is more or less the approach we take with error(), die(), etc, > > which are built on vreportf() and its fixed buffer. > > Would another approach be to add various trace2 functions that use > strbuf() allocation a way to tell if they are called from a signal > handing codepath, and punt (by doing nothing if needed, but > hopefully we have enough slop in the buffer to say "hey we got > interrupted so no more detailed report for you, sorry") if that is > the case? We do use that "in_signal" flag in other handlers. E.g., when run-command avoids calling free() in a signal, and the tempfile code avoids using stdio. But in the case of these trace functions, I think they'd all need to be rewritten to avoid strbufs. That message formatting is the whole point, and there is no way to have a strbuf which truncates rather than growing (though it is something we've discussed). So I think we either need to rip strbufs out of most of trace2, or let these signal paths re-implement the formatting in a super-simple way. -Peff
