Skip to content

varbuf: add optional -fbounds-safety annotations for struct ap_varbuf - #754

Open
LaptopsPlural wants to merge 3 commits into
apache:trunkfrom
LaptopsPlural:local/varbuf-fbounds-safety
Open

LaptopsPlural wants to merge 3 commits into
apache:trunkfrom
LaptopsPlural:local/varbuf-fbounds-safety

Conversation

@LaptopsPlural

@LaptopsPlural LaptopsPlural commented Sep 11, 2026 •

Copy link
Copy Markdown

Summary

Secure-by-design memory-safety hardening. Annotates struct ap_varbuf.buf with optional Clang -fbounds-safety / sized-by macros tied to avail. Default builds unchanged (opt-in OFF).

Contributor: Jeff Bindel via LaptopsPlural. Not a vulnerability PoC.

Test plan

  • Default CMake/autotools build
  • Optional bounds-safety ON with supporting Clang (maintainers)

Introduce inert AP_SIZED_BY*_ macros (OFF by default) and annotate the
ap_varbuf buf/avail pair. Capacity-first assign in large-grow/init/free.
Default builds unchanged; ENABLE_FBOUNDS_SAFETY / --enable-fbounds-safety
opt-in for experimental Clang toolchains.
@notroj

notroj commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Very nice! I think this would be better in ap_config.h - toward the end - there are already some compiler-specific attribute handling things there.

Per notroj's review, place the inert AP_SIZED_BY* / AP_COUNTED_BY*
macros next to the existing AP_FN_ATTR_* compiler attribute helpers
in ap_config.h and drop the standalone ap_bounds_safety.h.
@LaptopsPlural

Copy link
Copy Markdown
Author

@notroj Thanks — moved the inert macros into ap_config.h next to the existing attribute helpers and dropped the standalone ap_bounds_safety.h.

@notroj

notroj commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks!

Not trying to be awkward here but it looks like that LLVM feature is still a WIP? I was looking to see if there is something similar for GCC and found https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-counted_005fby - it's implied this would be used/usable under a UBSAN build already? (I wonder if we could try to trip it, we have UBSAN in CI already)

@LaptopsPlural

Copy link
Copy Markdown
Author

@notroj Good questions — you’re right on both counts.

Yes: Clang -fbounds-safety / __sized_by* is still experimental (Apple/LLVM WIP). That’s why this stays opt-in OFF by default and the AP_* macros in ap_config.h are inert unless someone explicitly turns ENABLE_FBOUNDS_SAFETY on with a toolchain that has <ptrcheck.h>.

GCC’s counted_by is the more mature cousin, and you’re also right that UBSAN can check it (so it’s attractive given httpd already has UBSAN in CI). Two practical mismatches for this tip though:

  1. Attribute shape. ap_varbuf.buf is annotated as a byte bound (avail + 1, including the NUL), which maps to Clang __sized_by_or_null. GCC counted_by is an element count. For char * those coincide numerically, but it’s a different attribute family than what this PR wires today.

  2. Field order / ABI. In the public struct ap_varbuf, buf is declared before avail. GCC’s counted_by wants the count field declared first. Flipping that order would be an ABI break, so we can’t just swap in __attribute__((counted_by(avail + 1))) on buf without a larger design change.

So I don’t think we can honestly “trip it in UBSAN CI” with a one-line GCC swap on this struct as it stands. What this PR is aiming for is: inert by default, optional Clang path for people with that toolchain, and capacity-before-pointer assigns so the invariants are correct if/when bounds checking is on.

Happy to trim the PR description so it doesn’t oversell the LLVM side, or adjust the macros/docs if you want the GCC/counted_by story called out more clearly for future work.

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.

2 participants