Skip to content

[libc++][hardening] Finish documenting hardening. #92021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
373 changes: 363 additions & 10 deletions libcxx/docs/Hardening.rst
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.. _hardening-modes:
.. _hardening:

===============
Hardening Modes
Expand Down Expand Up @@ -29,8 +29,11 @@ modes are:
rigour impacts performance more than fast mode: we recommend benchmarking to
determine if that is acceptable for your program.
- **Debug mode**, which enables all the available checks in the library,
including internal assertions, some of which might be very expensive. This
mode is intended to be used for testing, not in production.
including heuristic checks that might have significant performance overhead as
well as internal library assertions. This mode should be used in
non-production environments (such as test suites, CI, or local development).
We don’t commit to a particular level of performance in this mode and it’s
*not* intended to be used in production.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add:

However, we do not change the complexity of algorithms since satisfying the complexity of an algorithm is necessary for Standards conformance.

Or, if we don't want to commit to that, then we should mention the contrary. Either way, it seems like a nice place to document this design point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly suspect that not changing the complexity is the right approach, but I'm still very slightly hesitant to commit to it at this point, given that our focus has been mainly on the fast mode and not on the debug mode. Do you think we could simply write that we're not sure although leaning towards not changing? Or should we only document decisions that we're pretty sure about, to avoid confusing our users?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should document something like "we think we're going to do X but we're not certain". That doesn't add any clarity as far as users are concerned. If we're too unsure about what we want to do, I would recommend just not saying anything (i.e. status quo).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is absolutely worth documenting, but I'd prefer to do it at a later point after we spend more time restoring feature parity with the old debug mode and become more confident that this is the right approach.

.. note::

Expand Down Expand Up @@ -72,17 +75,367 @@ to control the level by passing **one** of the following options to the compiler
Notes for vendors
-----------------

Vendors can set the default hardening mode by providing ``LIBCXX_HARDENING_MODE``
as a configuration option, with the possible values of ``none``, ``fast``,
``extensive`` and ``debug``. The default value is ``none`` which doesn't enable
any hardening checks (this mode is sometimes called the ``unchecked`` mode).
Vendors can set the default hardening mode by providing
``LIBCXX_HARDENING_MODE`` as a configuration option, with the possible values of
``none``, ``fast``, ``extensive`` and ``debug``. The default value is ``none``
which doesn't enable any hardening checks (this mode is sometimes called the
``unchecked`` mode).

This option controls both the hardening mode that the precompiled library is
built with and the default hardening mode that users will build with. If set to
``none``, the precompiled library will not contain any assertions, and user code
will default to building without assertions.

Iterator bounds checking
------------------------
Vendors can also override the way the program is terminated when an assertion
fails by :ref:`providing a custom header <override-assertion-handler>`.

TODO(hardening)
Assertion categories
====================

Inside the library, individual assertions are grouped into different
*categories*. Each hardening mode enables a different set of assertion
categories; categories provide an additional layer of abstraction that makes it
easier to reason about the high-level semantics of a hardening mode.

.. note::

Users are not intended to interact with these categories directly -- the
categories are considered internal to the library and subject to change.

- ``valid-element-access`` -- checks that any attempts to access a container
element, whether through the container object or through an iterator, are
valid and do not attempt to go out of bounds or otherwise access
a non-existent element. This also includes operations that set up an imminent
invalid access (e.g. incrementing an end iterator). For iterator checks to
work, bounded iterators must be enabled in the ABI. Types like
``std::optional`` and ``std::function`` are considered containers (with at
most one element) for the purposes of this check.

- ``valid-input-range`` -- checks that ranges (whether expressed as an iterator
pair, an iterator and a sentinel, an iterator and a count, or
a ``std::range``) given as input to library functions are valid:
- the sentinel is reachable from the begin iterator;
- TODO(hardening): both iterators refer to the same container.

("input" here refers to "an input given to an algorithm", not to an iterator
category)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider renaming valid-input-range to valid-range-argument to avoid the possible confusion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 -- I'll prepare a follow-up.


Violating assertions in this category leads to an out-of-bounds access.

- ``non-null`` -- checks that the pointer being dereferenced is not null. On
most modern platforms, the zero address does not refer to an actual location
in memory, so a null pointer dereference would not compromise the memory
security of a program (however, it is still undefined behavior that can result
in strange errors due to compiler optimizations).

- ``non-overlapping-ranges`` -- for functions that take several ranges as
arguments, checks that those ranges do not overlap.

- ``valid-deallocation`` -- checks that an attempt to deallocate memory is valid
(e.g. the given object was allocated by the given allocator). Violating this
category typically results in a memory leak.

- ``valid-external-api-call`` -- checks that a call to an external API doesn't
fail in an unexpected manner. This includes triggering documented cases of
undefined behavior in an external library (like attempting to unlock an
unlocked mutex in pthreads). Any API external to the library falls under this
category (from system calls to compiler intrinsics). We generally don't expect
these failures to compromise memory safety or otherwise create an immediate
security issue.

- ``compatible-allocator`` -- checks any operations that exchange nodes between
containers to make sure the containers have compatible allocators.

- ``argument-within-domain`` -- checks that the given argument is within the
domain of valid arguments for the function. Violating this typically produces
an incorrect result (e.g. ``std::clamp`` returns the original value without
clamping it due to incorrect functors) or puts an object into an invalid state
(e.g. a string view where only a subset of elements is accessible). This
category is for assertions violating which doesn't cause any immediate issues
in the library -- whatever the consequences are, they will happen in the user
code.

- ``pedantic`` -- checks preconditions that are imposed by the Standard, but
violating which happens to be benign in libc++.

- ``semantic-requirement`` -- checks that the given argument satisfies the
semantic requirements imposed by the Standard. Typically, there is no simple
way to completely prove that a semantic requirement is satisfied; thus, this
would often be a heuristic check and it might be quite expensive.

- ``internal`` -- checks that internal invariants of the library hold. These
assertions don't depend on user input.

- ``uncategorized`` -- for assertions that haven't been properly classified yet.
This category is an escape hatch used for some existing assertions in the
library; all new code should have its assertions properly classified.

Mapping between the hardening modes and the assertion categories
================================================================

.. list-table::
:header-rows: 1
:widths: auto

* - Category name
- ``fast``
- ``extensive``
- ``debug``
* - ``valid-element-access``
- ✅
- ✅
- ✅
* - ``valid-input-range``
- ✅
- ✅
- ✅
* - ``non-null``
- ❌
- ✅
- ✅
* - ``non-overlapping-ranges``
- ❌
- ✅
- ✅
* - ``valid-deallocation``
- ❌
- ✅
- ✅
* - ``valid-external-api-call``
- ❌
- ✅
- ✅
* - ``compatible-allocator``
- ❌
- ✅
- ✅
* - ``argument-within-domain``
- ❌
- ✅
- ✅
* - ``pedantic``
- ❌
- ✅
- ✅
* - ``semantic-requirement``
- ❌
- ❌
- ✅
* - ``internal``
- ❌
- ❌
- ✅
* - ``uncategorized``
- ❌
- ✅
- ✅

.. note::

At the moment, each subsequent hardening mode is a strict superset of the
previous one (in other words, each subsequent mode only enables additional
assertion categories without disabling any), but this won't necessarily be
true for any hardening modes that might be added in the future.

.. note::

The categories enabled by each mode are subject to change and users should not
rely on the precise assertions enabled by a mode at a given point in time.
However, the library does guarantee to keep the hardening modes stable and
to fulfill the semantics documented here.

Hardening assertion failure
===========================

In production modes (``fast`` and ``extensive``), a hardening assertion failure
immediately ``_traps <https://llvm.org/docs/LangRef.html#llvm-trap-intrinsic>``
the program. This is the safest approach that also minimizes the code size
penalty as the failure handler maps to a single instruction. The downside is
that the failure provides no additional details other than the stack trace
(which might also be affected by optimizations).

TODO(hardening): describe ``__builtin_verbose_trap`` once we can use it.

In the ``debug`` mode, an assertion failure terminates the program in an
unspecified manner and also outputs the associated error message to the error
output. This is less secure and increases the size of the binary (among other
things, it has to store the error message strings) but makes the failure easier
to debug. It also allows testing the error messages in our test suite.
Comment on lines +261 to +262
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
output. This is less secure and increases the size of the binary (among other
things, it has to store the error message strings) but makes the failure easier
output. This is less secure and increases the size of the binary (since it has to store the error messages inside the compiled binary) but makes the failure easier

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, in addition to storing messages, the whole mechanism used to output error messages can increase the code size (additional instructions to do the output, plus the fact that we're doing a function call which might not always be inlined). Do you think we should ignore that for the purposes of this section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your current formulation is acceptable. I initially had trouble parsing it cause I missed the beginning of the ( but I think status quo is fine.


.. _override-assertion-handler:

Overriding the assertion failure handler
----------------------------------------

Vendors can override the default assertion handler mechanism by following these
steps:

- create a header file that provides a definition of a macro called
``_LIBCPP_ASSERTION_HANDLER``. The macro will be invoked when a hardening
assertion fails, with a single parameter containing a null-terminated string
with the error message.
- when configuring the library, provide the path to custom header (relative to
the root of the repository) via the CMake variable
``LIBCXX_ASSERTION_HANDLER_FILE``.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is the right place to mention that the assertion handler gets included everywhere, so there must not be dependencies on other parts of the library in that header.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Should we mention that __config is an exception? (Not sure if we're okay with vendors including it since it's a private header)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could say should not include anything non trivial to keep things somewhat vague. I'm also fine with mentioning __config directly, it just seems a bit too restrictive to prevent including anything else (since several headers don't have a dependency on the handler). Basically I think the intent we should go for is to warn vendors about the potential for circular deps and then let them figure out something that works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with should not include anything non trivial (it seems easy enough to use the default header as an example and notice that it does include __config).

Note that almost all libc++ headers include the assertion handler header which
means it should not include anything non-trivial from the standard library to
avoid creating circular dependencies.

There is no existing mechanism for users to override the assertion handler
because the ability to do the override other than at configure-time carries an
unavoidable code size penalty that would otherwise be imposed on all users,
whether they require such customization or not. Instead, we let vendors decide
what's right on their platform for their users -- a vendor who wishes to provide
this capability is free to do so, e.g. by declaring the assertion handler as an
overridable function.

ABI
===

Setting a hardening mode does **not** affect the ABI. Each mode uses the subset
of checks available in the current ABI configuration which is determined by the
platform.

It is important to stress that whether a particular check is enabled depends on
the combination of the selected hardening mode and the hardening-related ABI
options. Some checks require changing the ABI from the "default" to store
additional information in the library classes -- e.g. checking whether an
iterator is valid upon dereference generally requires storing data about bounds
inside the iterator object. Using ``std::span`` as an example, setting the
hardening mode to ``fast`` will always enable the ``valid-element-access``
checks when accessing elements via a ``std::span`` object, but whether
dereferencing a ``std::span`` iterator does the equivalent check depends on the
ABI configuration.

ABI options
-----------

Vendors can use the following ABI options to enable additional hardening checks:

- ``_LIBCPP_ABI_BOUNDED_ITERATORS`` -- changes the iterator type of select
containers (see below) to a bounded iterator that keeps track of whether it's
within the bounds of the original container and asserts valid bounds on every
dereference.

ABI impact: changes the iterator type of the relevant containers.

Supported containers:

- ``span``;
- ``string_view``.

ABI tags
--------

We use ABI tags to allow translation units built with different hardening modes
to interact with each other without causing ODR violations. Knowing how
hardening modes are encoded into the ABI tags might be useful to examine
a binary and determine whether it was built with hardening enabled.

.. warning::
We don't commit to the encoding scheme used by the ABI tags being stable
between different releases of libc++. The tags themselves are never stable, by
design -- new releases increase the version number. The following describes
the state of the latest release and is for informational purposes only.

The first character of an ABI tag encodes the hardening mode:

- ``f`` -- [f]ast mode;
- ``s`` -- extensive ("[s]afe") mode;
- ``d`` -- [d]ebug mode;
- ``n`` -- [n]one mode.

Comment on lines +328 to +347
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am worried that this is going to become stale very quickly.

I do agree it is useful for users to be able to know the configuration they're looking at. I'm conflicted between keeping this section and removing it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do share this concern. I tried to stress that in the warning note, do you think emphasizing this even more would help?

Hardened containers status
==========================

.. list-table::
:header-rows: 1
:widths: auto

* - Name
- Member functions
- Iterators (ABI-dependent)
* - ``span``
- ✅
- ✅
* - ``string_view``
- ✅
- ✅
* - ``array``
- ✅
- ❌
* - ``vector``
- ✅
- ❌
* - ``string``
- ✅
- ❌
* - ``list``
- ✅
- ❌
* - ``forward_list``
- ❌
- ❌
* - ``deque``
- ✅
- ❌
* - ``map``
- ❌
- ❌
* - ``set``
- ❌
- ❌
* - ``multimap``
- ❌
- ❌
* - ``multiset``
- ❌
- ❌
* - ``unordered_map``
- Partial
- Partial
* - ``unordered_set``
- Partial
- Partial
* - ``unordered_multimap``
- Partial
- Partial
* - ``unordered_multiset``
- Partial
- Partial
* - ``mdspan``
- ✅
- ❌
* - ``optional``
- ✅
- N/A
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I miss

  • map, set, and their multi and unordered parts. (we don't have the flat ones)
  • valarray
  • bitset (this has no iterators, but has an index operation.
  • the container adapters, stack, queue and priority_queue
  • function was listed above
  • what about variant, any, and expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Would you be okay if I do this in an (immediate) follow-up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that would be fine by me.

* - ``function``
- ❌
- N/A
* - ``variant``
- N/A
- N/A
* - ``any``
- N/A
- N/A
* - ``expected``
- ✅
- N/A
* - ``valarray``
- Partial
- N/A
* - ``bitset``
- ❌
- N/A

Testing
=======

Please see :ref:`Testing documentation <testing-hardening-assertions>`.

Further reading
===============

- ``_Hardening RFC <https://discourse.llvm.org/t/rfc-hardening-in-libc/73925>``:
contains some of the design rationale.
Loading
Loading