Restrict malloc of uninitialized memory containing checked pointers?

[secure-sw-dev-bot]

This issue was copied from checkedc/checkedc#458

---

Section 2.6 of the Checked C specification states:

For heap-allocated data that contains checked pointers that may be used to access memory, the data must be zero-initialized by the programmer. We recommend that programmers use calloc instead of malloc for heap-allocating data containing checked pointers.

I know this is a known issue, but it's not ideal to leave random soundness holes lying around like this and rely on the programmer to notice whenever their code is doing something dangerous and fix it. It would easy to overlook a malloc call when porting a large C program to Checked C. Is there any way that malloc of checked pointers could be restricted better?

Some potential approaches:

1. Just remove the bounds-safe declaration for malloc, forcing checked regions to use calloc instead. That would be disruptive to existing Checked C code, but if we care enough about soundness, maybe the disruption is justified.

2. Place a constraint on the declaration of malloc that the type parameter T cannot contain checked pointers (the same condition the compiler checks for uninitialized stack-allocated variables). That has the advantage of only breaking existing code that is actually unsafe, but it requires a new language feature that probably wouldn't be used for anything except malloc, which probably isn't worth it.

3. Change the declaration of malloc to:

   void *malloc(size_t size) : itype(_Array_ptr<void>) byte_count(size);

   and let the caller do an implicit conversion from _Array_ptr<void> : byte_count(size) to _Array_ptr<T> : byte_count(size). This takes advantage of the compiler's existing logic that allows such a conversion only if T does not contain checked pointers. This would break existing calls to malloc until the generic type arguments are removed, unless we leave a dummy _Itype_for_any(T) on the declaration of malloc, which would buy us compatibility in the short term but be silly in the long term.

In all the above approaches, the compatibility issues mainly affect existing Checked C code. I don't think any of the approaches are materially worse for porting existing plain C code than the status quo, under which the user has to modify malloc calls to add a type argument.

In combination with any of the approaches for restricting traditional malloc calls that don't initialize the memory, one thing we could offer programmers to reduce disruption is a function (which I'll call zalloc for the purpose of this discussion) that is defined in checkedc_extensions.h and has the same signature as malloc (unlike calloc, which takes an extra parameter) but zero-initializes the memory. Then programmers would have the option to #define malloc zalloc, and their Checked C code should work as before.

realloc poses the same soundness problem as malloc, and the same options are available for restricting it. Unfortunately, providing a safe alternative to realloc (called zrealloc in this discussion) is more difficult: in order to zero-initialize only the added memory, we need to know the old size of the memory block. The C library doesn't provide a standard way to query the old size, but zrealloc could probably take it as an extra parameter, since in Checked C, the caller will generally have this information anyway as part of the bounds declaration of the old pointer. So the declaration and implementation of zrealloc in checkedc_extensions.h could look something like this:

inline __attribute__((__always_inline__))
_Itype_for_any(T) void *zrealloc(
    void *pointer : itype(_Array_ptr<T>) byte_count(old_size),
    size_t new_size,
    size_t old_size _Where old_size % sizeof(T) == 0)
    : itype(_Array_ptr<T>) byte_count(new_size) _Unchecked {
  void *new_pointer = realloc(pointer, new_size);
  if (new_size > old_size) {
    memset(new_pointer + old_size, 0, new_size - old_size);
  }
  return new_pointer;
}

Note the _Where clause: we mustn't allow the user to zero out part of a T object because that could create an invalid pointer, a struct where a pointer member is inconsistent with its bounds member, etc. The compiler currently doesn't support sizeof(T) in a _Where clause; that's tracked as part of #454.

(Side note: For (1), as an alternative to completely removing the bounds-safe declaration for malloc, I tried just marking it _Unchecked, thinking that would disallow calls from checked regions while still providing bounds information to help catch bounds-related mistakes on calls from unchecked regions. But in my tests with the current compiler, marking a function declaration _Unchecked does not prevent calls from checked regions. Is that a separate bug that I should report?)