Skip to content

Commit 6941851

Browse files
committed
Merge pull request #1554 from bettio/fix-gc-error-in-ets_update_counter
Fix memory corruption in `ets_update_counter` Fixes #1553 These changes are made under both the "Apache 2.0" and the "GNU Lesser General Public License 2.1 or later" license terms (dual license). SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
2 parents 858cee8 + 041dd65 commit 6941851

File tree

3 files changed

+24
-16
lines changed

3 files changed

+24
-16
lines changed

src/libAtomVM/ets.c

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ void ets_destroy(struct Ets *ets, GlobalContext *global)
144144
synclist_destroy(&ets->ets_tables);
145145
}
146146

147-
EtsErrorCode ets_create_table(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx)
147+
EtsErrorCode ets_create_table_maybe_gc(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx)
148148
{
149149
if (is_named) {
150150
struct EtsTable *ets_table = ets_get_table_by_name(&ctx->global->ets, name, TableAccessNone);
@@ -348,7 +348,7 @@ EtsErrorCode ets_insert(term name_or_ref, term entry, Context *ctx)
348348
return result;
349349
}
350350

351-
static EtsErrorCode ets_table_lookup(struct EtsTable *ets_table, term key, term *ret, Context *ctx)
351+
static EtsErrorCode ets_table_lookup_maybe_gc(struct EtsTable *ets_table, term key, term *ret, Context *ctx, int num_roots, term *roots)
352352
{
353353
if (ets_table->access_type == EtsAccessPrivate && ets_table->owner_process_id != ctx->process_id) {
354354
return EtsPermissionDenied;
@@ -362,7 +362,7 @@ static EtsErrorCode ets_table_lookup(struct EtsTable *ets_table, term key, term
362362

363363
size_t size = (size_t) memory_estimate_usage(res);
364364
// allocate [object]
365-
if (UNLIKELY(memory_ensure_free_opt(ctx, size + CONS_SIZE, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
365+
if (UNLIKELY(memory_ensure_free_with_roots(ctx, size + CONS_SIZE, num_roots, roots, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
366366
return EtsAllocationFailure;
367367
}
368368
term new_res = memory_copy_term_tree(&ctx->heap, res);
@@ -372,20 +372,20 @@ static EtsErrorCode ets_table_lookup(struct EtsTable *ets_table, term key, term
372372
return EtsOk;
373373
}
374374

375-
EtsErrorCode ets_lookup(term name_or_ref, term key, term *ret, Context *ctx)
375+
EtsErrorCode ets_lookup_maybe_gc(term name_or_ref, term key, term *ret, Context *ctx)
376376
{
377377
struct EtsTable *ets_table = term_is_atom(name_or_ref) ? ets_get_table_by_name(&ctx->global->ets, name_or_ref, TableAccessRead) : ets_get_table_by_ref(&ctx->global->ets, term_to_ref_ticks(name_or_ref), TableAccessRead);
378378
if (ets_table == NULL) {
379379
return EtsTableNotFound;
380380
}
381381

382-
EtsErrorCode result = ets_table_lookup(ets_table, key, ret, ctx);
382+
EtsErrorCode result = ets_table_lookup_maybe_gc(ets_table, key, ret, ctx, 0, NULL);
383383
SMP_UNLOCK(ets_table);
384384

385385
return result;
386386
}
387387

388-
EtsErrorCode ets_lookup_element(term name_or_ref, term key, size_t pos, term *ret, Context *ctx)
388+
EtsErrorCode ets_lookup_element_maybe_gc(term name_or_ref, term key, size_t pos, term *ret, Context *ctx)
389389
{
390390
if (UNLIKELY(pos == 0)) {
391391
return EtsBadPosition;
@@ -492,7 +492,7 @@ static bool operation_to_tuple4(term operation, size_t default_pos, term *positi
492492
return true;
493493
}
494494

495-
EtsErrorCode ets_update_counter(term ref, term key, term operation, term default_value, term *ret, Context *ctx)
495+
EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term operation, term default_value, term *ret, Context *ctx)
496496
{
497497
struct EtsTable *ets_table = term_is_atom(ref)
498498
? ets_get_table_by_name(&ctx->global->ets, ref, TableAccessWrite)
@@ -501,13 +501,21 @@ EtsErrorCode ets_update_counter(term ref, term key, term operation, term default
501501
return EtsTableNotFound;
502502
}
503503

504+
// do not use an invalid term as a root
505+
term safe_default_value = term_is_invalid_term(default_value) ? term_nil() : default_value;
506+
term roots[] = { key, operation, safe_default_value };
507+
504508
term list;
505-
EtsErrorCode result = ets_table_lookup(ets_table, key, &list, ctx);
509+
EtsErrorCode result = ets_table_lookup_maybe_gc(ets_table, key, &list, ctx, 3, roots);
506510
if (UNLIKELY(result != EtsOk)) {
507511
SMP_UNLOCK(ets_table);
508512
return result;
509513
}
510514

515+
key = roots[0];
516+
operation = roots[1];
517+
default_value = term_is_invalid_term(default_value) ? term_invalid_term() : roots[2];
518+
511519
term to_insert;
512520
if (term_is_nil(list)) {
513521
if (term_is_invalid_term(default_value)) {

src/libAtomVM/ets.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ struct Ets
7171
void ets_init(struct Ets *ets);
7272
void ets_destroy(struct Ets *ets, GlobalContext *global);
7373

74-
EtsErrorCode ets_create_table(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx);
74+
EtsErrorCode ets_create_table_maybe_gc(term name, bool is_named, EtsTableType table_type, EtsAccessType access_type, size_t keypos, term *ret, Context *ctx);
7575
void ets_delete_owned_tables(struct Ets *ets, int32_t process_id, GlobalContext *global);
7676

7777
EtsErrorCode ets_insert(term ref, term entry, Context *ctx);
78-
EtsErrorCode ets_lookup(term ref, term key, term *ret, Context *ctx);
79-
EtsErrorCode ets_lookup_element(term ref, term key, size_t pos, term *ret, Context *ctx);
78+
EtsErrorCode ets_lookup_maybe_gc(term ref, term key, term *ret, Context *ctx);
79+
EtsErrorCode ets_lookup_element_maybe_gc(term ref, term key, size_t pos, term *ret, Context *ctx);
8080
EtsErrorCode ets_delete(term ref, term key, term *ret, Context *ctx);
81-
EtsErrorCode ets_update_counter(term ref, term key, term value, term pos, term *ret, Context *ctx);
81+
EtsErrorCode ets_update_counter_maybe_gc(term ref, term key, term value, term pos, term *ret, Context *ctx);
8282
#ifdef __cplusplus
8383
}
8484
#endif

src/libAtomVM/nifs.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3192,7 +3192,7 @@ static term nif_ets_new(Context *ctx, int argc, term argv[])
31923192
}
31933193

31943194
term table = term_invalid_term();
3195-
EtsErrorCode result = ets_create_table(name, is_named == TRUE_ATOM, EtsTableSet, access, term_to_int(keypos) - 1, &table, ctx);
3195+
EtsErrorCode result = ets_create_table_maybe_gc(name, is_named == TRUE_ATOM, EtsTableSet, access, term_to_int(keypos) - 1, &table, ctx);
31963196
switch (result) {
31973197
case EtsOk:
31983198
return table;
@@ -3244,7 +3244,7 @@ static term nif_ets_lookup(Context *ctx, int argc, term argv[])
32443244
term key = argv[1];
32453245

32463246
term ret = term_invalid_term();
3247-
EtsErrorCode result = ets_lookup(ref, key, &ret, ctx);
3247+
EtsErrorCode result = ets_lookup_maybe_gc(ref, key, &ret, ctx);
32483248
switch (result) {
32493249
case EtsOk:
32503250
return ret;
@@ -3270,7 +3270,7 @@ static term nif_ets_lookup_element(Context *ctx, int argc, term argv[])
32703270
VALIDATE_VALUE(pos, term_is_integer);
32713271

32723272
term ret = term_invalid_term();
3273-
EtsErrorCode result = ets_lookup_element(ref, key, term_to_int(pos), &ret, ctx);
3273+
EtsErrorCode result = ets_lookup_element_maybe_gc(ref, key, term_to_int(pos), &ret, ctx);
32743274
switch (result) {
32753275
case EtsOk:
32763276
return ret;
@@ -3326,7 +3326,7 @@ static term nif_ets_update_counter(Context *ctx, int argc, term argv[])
33263326
default_value = term_invalid_term();
33273327
}
33283328
term ret;
3329-
EtsErrorCode result = ets_update_counter(ref, key, operation, default_value, &ret, ctx);
3329+
EtsErrorCode result = ets_update_counter_maybe_gc(ref, key, operation, default_value, &ret, ctx);
33303330
switch (result) {
33313331
case EtsOk:
33323332
return ret;

0 commit comments

Comments
 (0)