Skip to content

Fix IPv6 diff boundary bug and update test harnesses#41

Merged
ktsaou merged 1 commit intomasterfrom
review-fixes
Apr 4, 2026
Merged

Fix IPv6 diff boundary bug and update test harnesses#41
ktsaou merged 1 commit intomasterfrom
review-fixes

Conversation

@ktsaou
Copy link
Copy Markdown
Member

@ktsaou ktsaou commented Apr 4, 2026

Summary

Findings from 4-model parallel code review (GLM-5.1, Kimi K2.5, Qwen 3.5+, MiniMax M2.7) + ASAN/UBSAN/TSAN + valgrind memcheck.

Bug fix:

  • ipset6_diff.c: fix stale lo1/hi1 (and lo2/hi2) after i1++/i2++ at the IPV6_ADDR_MAX boundary. The conditional refresh lo1 > hi1 could leave values from the prior iteration, producing incorrect diff results for IPv6 ranges ending at the maximum address.

Defensive fix:

  • ipset_dns.c: use memset for addrinfo hints struct, consistent with the IPv6 DNS resolver.

Test harness fixes:

  • Sanitizer test 05 (DNS thread-create failure): add ipset_dns.c to compilation, define active_family/ipv6_dropped_in_ipv4_mode globals.
  • All 6 unit test harnesses: add the same missing globals.
  • run-unit-tests.sh: add ipset_dns.c to PROJECT_SOURCES.

Verification

  • 93/93 CLI tests pass
  • 7/7 build tests pass
  • 5/5 sanitizer CLI tests pass (was 4/5 before fix)
  • 6/6 unit tests pass under ASAN/UBSAN (was 0/6 before fix)
  • 1/1 TSAN test passes
  • 27/28 valgrind scenarios clean (1 glibc resolver false positive)

Review triage

Findings verified against code, dispositioned:

Finding Verdict Action
IPv6 diff stale lo1/hi1 at MAX boundary VALID Fixed
DNS thread race on dns_threads Valid but harmless (single-threaded caller) No action
Memory leak in binary_validate_payload False positive (tmp freed before check) No action
Uninitialized addrinfo hints Partially valid Fixed (memset)
File descriptor leak in ipset_load.c False positive (fp closed before dns_done) No action
Buffer overflow in parse_hostname False positive (i < len check correct) No action
IPv6 unique_ips overflow Not a bug (deliberate saturation, documented) No action
IPv6 exclude tail copy bug False positive (pattern is correct) No action

- Fix ipset6_diff.c: always refresh lo1/hi1 and lo2/hi2 from the array
  after incrementing i1/i2 at the IPV6_ADDR_MAX boundary. The previous
  conditional refresh (lo1 > hi1) could leave stale values from the
  prior iteration, producing incorrect diff results for ranges ending
  at the maximum IPv6 address.

- Use memset for addrinfo hints in ipset_dns.c for consistency with
  the IPv6 DNS resolver (defensive, avoids uninitialized struct fields).

- Fix sanitizer test 05 and unit test harnesses: add ipset_dns.c to
  the compilation and define active_family/ipv6_dropped_in_ipv4_mode
  globals required after the DNS extraction and IPv6 additions.
@ktsaou ktsaou merged commit e633f70 into master Apr 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant