Multiple double-frees on table-entry allocation failure in OpenTelemetry encoder
- Dominant language
- C
- Stars
- 0
- Forks
- 1
- Avg merge
- 3d 11h
- Merged PRs (30d)
- 1
Description
## Summary
`build_profiles_dictionary()` explicitly frees several dictionary table
allocations when allocation of their required zero-value entry fails, and then
calls `destroy_profiles_dictionary()`. The destructor still sees the original
non-`NULL` pointer and frees the same allocation a second time.
The issue affects `stack_table`, `mapping_table`, `location_table`,
`function_table`, and `link_table`.
This is an allocation-failure cleanup bug in the OpenTelemetry encoder. It is
in the CProfiles repository and should be fixed and coordinated here, with a
cross-reference from downstream consumers such as Fluent Bit if appropriate.
## Affected code
In the current repository tree:
- `src/cprof_encode_opentelemetry.c:865-917` —
`destroy_profiles_dictionary()` frees all of these dictionary tables.
- `src/cprof_encode_opentelemetry.c:1447-1517` —
`build_profiles_dictionary()` allocates each table and its required
zero-value entry.
Table | Entry allocation fails | Explicit table free | Destructor table free
-- | -- | -- | --
stack_table | 1453-1454 | 1455 | 911-915
mapping_table | 1468-1469 | 1470 | 873-877
location_table | 1483-1484 | 1485 | 879-883
function_table | 1498-1499 | 1500 | 885-889
link_table | 1513-1514 | 1515 | 891-895
The `string_table[0]` failure at lines 1439-1443 uses a separate cleanup path
that frees the table and `dict` directly, so it does not exhibit this specific
double-free pattern.
## Failure path
For each affected table, the sequence is:
1. The table pointer is allocated.
2. Allocation of its `[0]` zero-value entry fails.
3. The failure branch explicitly frees the table pointer.
4. The same branch calls `destroy_profiles_dictionary(dict)`.
5. Because the table pointer was not set to `NULL`, the destructor enters its
corresponding cleanup block and frees the table pointer again.
The cleanup call also iterates up to the corresponding `n_*_table` count. For
the table whose entry allocation failed, that count is still zero, so the
immediate issue is the stale table pointer being freed twice. Previously
initialized tables are cleaned up normally by the same destructor call.
## Impact and severity
Severity: **Low — OOM-only availability and allocator-integrity bug**.
The encoder can abort or corrupt allocator state while handling an allocation
failure. The issue requires one of the table-pointer allocations to succeed
while the corresponding zero-value object allocation fails, such as through
out-of-memory or deterministic allocator fault injection. No ordinary-input
use-after-free or remote code execution has been demonstrated.
## Reproduction status
Not reproduced with a normal configuration. Reproduction requires deterministic
allocation failure at one of the zero-value entry `calloc()` calls on lines
1453, 1468, 1483, 1498, or 1513.
## Proposed fix
Use `destroy_profiles_dictionary()` as the sole owner of all five table
allocations in these failure paths by removing the explicit `free()` calls:
```diff
diff --git a/src/cprof_encode_opentelemetry.c b/src/cprof_encode_opentelemetry.c
@@ -1452,7 +1452,6 @@ static int build_profiles_dictionary(
}
dict->stack_table[0] = calloc(1, sizeof(Opentelemetry__Proto__Profiles__V1development__Stack));
if (dict->stack_table[0] == NULL) {
- free(dict->stack_table);
destroy_profiles_dictionary(dict);
return CPROF_ENCODE_OPENTELEMETRY_ALLOCATION_ERROR;
}
@@ -1467,7 +1467,6 @@ static int build_profiles_dictionary(
}
dict->mapping_table[0] = calloc(1, sizeof(Opentelemetry__Proto__Profiles__V1development__Mapping));
if (dict->mapping_table[0] == NULL) {
- free(dict->mapping_table);
destroy_profiles_dictionary(dict);
return CPROF_ENCODE_OPENTELEMETRY_ALLOCATION_ERROR;
}
@@ -1482,7 +1482,6 @@ static int build_profiles_dictionary(
}
dict->location_table[0] = calloc(1, sizeof(Opentelemetry__Proto__Profiles__V1development__Location));
if (dict->location_table[0] == NULL) {
- free(dict->location_table);
destroy_profiles_dictionary(dict);
return CPROF_ENCODE_OPENTELEMETRY_ALLOCATION_ERROR;
}
@@ -1497,7 +1497,6 @@ static int build_profiles_dictionary(
}
dict->function_table[0] = calloc(1, sizeof(Opentelemetry__Proto__Profiles__V1development__Function));
if (dict->function_table[0] == NULL) {
- free(dict->function_table);
destroy_profiles_dictionary(dict);
return CPROF_ENCODE_OPENTELEMETRY_ALLOCATION_ERROR;
}
@@ -1512,7 +1512,6 @@ static int build_profiles_dictionary(
}
dict->link_table[0] = calloc(1, sizeof(Opentelemetry__Proto__Profiles__V1development__Link));
if (dict->link_table[0] == NULL) {
- free(dict->link_table);
destroy_profiles_dictionary(dict);
return CPROF_ENCODE_OPENTELEMETRY_ALLOCATION_ERROR;
}
```
An alternative is to set each table pointer to `NULL` immediately after its
explicit `free()`, but keeping one cleanup owner is less error-prone.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/cprof_encode_opentelemetry.c by reading destroy_profiles_dictionary() around lines 865-917 and build_profiles_dictionary() around lines 1447-1517. Check each zero-value entry allocation failure path for duplicate ownership of the five table pointers. Done means those paths have one cleanup owner and no table allocation can be freed twice.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- observability
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100