Hi Karan, On Tue, Jul 21, 2026 at 10:43:42PM +0530, Karan Kurani wrote: > This is a design question, not a patch series. I have not started an > implementation and would like maintainer direction before doing so. > > While reviewing the libdw DWARF parsers, I noticed an asymmetry in the > reading layer. > > get_uleb128/get_sleb128 take an end pointer, and explicitly named unchecked > LEB variants exist for already validated input. The incrementing fixed-width > readers, however, such as: > > read_2ubyte_unaligned_inc > read_3ubyte_unaligned_inc > read_4ubyte_unaligned_inc > read_8ubyte_unaligned_inc > read_addr_unaligned_inc > > do not take an end pointer. Each caller must perform its own bounds check > before the read. This creates a caller-discipline requirement that has > occasionally been missed.
There are also non-increasing read_xubyte_unaligned and not-converting (little/big endian) read_Xubyte_unaligned_noncvt variants. And of course some code just assumes it doesn't need converting and is reading aligned types. > There are also related one-byte cursor cases, such as minimum_instr_len via > *linep++ and the CFI advance_loc1 fix via *program++. These have a similar > caller-side boundary-check requirement but are not uses of the fixed-width > reader macros. Right. > A preliminary source census found about 42 incrementing fixed-width read > call > sites in libdw. For the sites reviewed, an end, limit, bytes_end, or > equivalent > boundary was already available in the surrounding parser, so a checked > reader > would not appear to require new boundary plumbing. > > One possible shape, expressed in existing project style rather than as a new > public API, would be: > > - a checked fixed-width reader receives the cursor and end boundary; > - a short read follows the caller's existing goto/error path, similar to > cfi_assert or "goto invalid"; > - the cursor advances only after a complete successful read; > - explicitly named _unchecked variants remain for proven prevalidated > paths, > mirroring the existing LEB checked/unchecked split; > - migration could be staged, starting with historically affected parsers; > - the current ambiguous raw forms could eventually be restricted if that > is > considered desirable. > > Valid-input behavior and the public ABI/API should remain unchanged. The > intent > would only be to reject truncated input earlier and more uniformly. Any > implementation would also include before/after measurements for line-program > and CFI parsing. > > I would appreciate your guidance on the following: > > 1. Would you prefer a central checked fixed-width reader family, or > continued > explicit caller-side checks? I wouldn't be against a new checked macro/function variant for the unaligned "inc" macros. But the current code does use explicit caller-side checks that combine easily with other access/read functions/macros. Converting might catch some missing caller-side checks, but I am somewhat optimistic (naive?) hoping the recent audits have caught most of them. I am a little conservative and so wouldn't do this myself. But if someone else does all the hard work I won't mind reviewing the result. > 2. Should failure use a cfi_assert/goto-style macro, a bool-returning > inline > helper, or another existing project idiom? I think a boolean returning inline function/macro would be the most natural. You can write a assert/goto-style macro on top of that where it makes sense. > 3. Should an initial effort cover only libdw, or all users of these shared > macros, including libdwfl, readelf and libcpu? It can be done in stages. > 4. Would a checked-by-default plus explicitly named _unchecked split be > acceptable, following the existing LEB design? Sure. > 5. Would you prefer one complete conversion or a staged series beginning > with > the historically affected parser families? Do it in as small stages as possible. > If you would rather continue handling these checks locally on a case-by-case > basis, that is also a perfectly reasonable answer, and I will not pursue the > larger change. Personally I just keep the local checks. I am not really convinced doing just the unaligned_inc variants will cover it all. But if you think it is useful then please go for it. Do it in small steps and we can see who is right. Cheers, Mark
