Fix destruct ub and ubsan ci#16
Merged
Merged
Conversation
igudich
force-pushed
the
fix-destruct-ub-and-ubsan-ci
branch
from
7ed69b1 to
07f98d8
Compare
Contributor
|
Nice, I will add some more sanitizers to the fuzz targets. You can just rebase on master an merge, |
jeso-mchp
approved these changes
May 20, 2026
added 4 commits
May 21, 2026 12:39
name##_free called destruct_free(f, &name##_destruct), which cast the typed destructor (e.g. void (*)(field_t *)) to void (*)(void *) and called through the latter. Per C99/C11 §6.3.2.3¶8, calling a function through a pointer of an incompatible type is undefined behaviour, even when the calling convention happens to match. clang's -fsanitize=function flags this on every *_free. Inline the destructor call in the macro body so the call goes through the function's actual type. destruct_free and destruct_cb_t become dead; remove them.
Build libef and ef-tests with -fsanitize=undefined -fno-sanitize-recover=all under TEST_ENABLE so any UB hit at runtime aborts the test and fails CI. Probe for -fsanitize=function (clang-only) and add it when available; this catches indirect-call type mismatches like the destruct_free regression fixed in the previous commit. Add a [gcc, clang] matrix to the GitHub Actions workflow so the function check actually runs in CI. Add test/test-alloc-free.cxx exercising every name##_free path explicitly (field, hdr, frame — including the recursive frame_free -> hdr_free -> field_free chain on a populated frame) so UBSan covers them.
Default ON preserves existing behaviour. -DEF_USE_LIBPCAP=OFF skips the detect-and-link block entirely so libef and ef do not pull in libpcap or its transitive deps. Useful for sanitizer builds where a transitive dependency (e.g. libnl-route-3 brought in by libpcap) trips MSan on uninitialized DSO-init reads outside our control — previously fixable only by carrying a downstream patch that commented out the detection block.
frame_def's serialization loop has the same shape as hdr_copy_to_buf_ which 4d9ce68 just guarded: if a frame_fill_defaults hook zeroes a field's bit_width while leaving its bit_offset past the new header end, hdr_write_field would assert. Today no header sets a def_val on a width-0 field, so this is purely defensive — but symmetric with the call-site fix and harmless. Also add a Catch2 regression test mirroring the ef-args.c sequence that triggered the original mld_fill_defaults bug: parse an MLDv2 query (with qrv/qqic/ns set, no rresv/ng), call frame_to_buf to drive mld_fill_defaults' shrink + bit_width-zeroing, then call frame_mask_to_buf on the same now-mutated frame. Pre-fix the second call aborted in hdr_write_field; post-fix the suite passes.
igudich
force-pushed
the
fix-destruct-ub-and-ubsan-ci
branch
from
07f98d8 to
fe12a52
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix UB in GEN_ALLOC_CLONE_FREE and run tests under UBSan
Summary
name##_freecalleddestruct_free(f, &name##_destruct), casting a typeddestructor (e.g.
void (*)(field_t *)) tovoid (*)(void *)and callingthrough the latter. C99/C11 §6.3.2.3¶8 makes this UB even when the calling
convention happens to match; clang's
-fsanitize=functionflags it onevery
*_free. Inline the destructor call in the macro so the call goesthrough the function's actual type;
destruct_free/destruct_cb_tbecome dead and are removed.
-fsanitize=undefined -fno-sanitize-recover=all,through the latter. C99/C11 §6.3.2.3¶8 makes this UB even when the calling
convention happens to match; clang's
-fsanitize=functionflags it onevery
*_free. Inline the destructor call in the macro so the call goesthrough the function's actual type;
destruct_free/destruct_cb_tbecome dead and are removed.
-fsanitize=undefined -fno-sanitize-recover=all,conditionally adding
-fsanitize=functionwhen the compiler supports it(clang). Any UB hit aborts the test and fails CI.
[gcc, clang]matrix so thefunction check actually runs.
test/test-alloc-free.cxxexercising everyname##_freepath(field / hdr / frame, including the recursive chain) so UBSan covers them.