From e760d19017a8e933ea952d5185f250ce23753fa9 Mon Sep 17 00:00:00 2001 From: lgcosta Date: Thu, 12 Nov 2026 09:11:55 -0300 Subject: [PATCH] Fix second use-after-free in the rollback path; clang battery re-validated ASan with halt_on_error caught a second dangling read on the exact path --rollback-fsck-changes exits through: fsck_rollback() called freemem(fs) at fsck/main.c:1313 and then printed the closing banner via fsck_progress(...), which dereferences fs (fsck_data(fs) etc.). Print the banner before the cleanup, matching the order the_end() and the rebuild paths already use. No behavior change otherwise -- the same bytes are emitted, only before the free. Re-validated after the fix: - clang+ASan/UBSan with halt_on_error: rollback save/restore, garbage-magic rejection, resize 64->128 MiB, mkfs, check, tune, debugreiserfs -- zero sanitizer errors; restore still prints "blocks restored" and the final banner. - host functional battery (gcc, --enable-warnings): 12/12. - host clang out-of-tree build: 0 warnings; clang rollback path exercised without sanitizers: exit semantics unchanged. - Debian VM, CC=clang battery on this exact tree: 12/12, 0 warnings. tests/vm/RESULTS.md documents the finding (item 6 in the clang section). --- fsck/main.c | 11 +++++++---- tests/vm/RESULTS.md | 21 +++++++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fsck/main.c b/fsck/main.c index 4edc8b6..8b52ca3 100644 --- a/fsck/main.c +++ b/fsck/main.c @@ -1303,16 +1303,19 @@ static void fsck_rollback(reiserfs_filsys_t fs) fsck_progress_file(fs)); close_rollback_file(); + /* print the closing banner while fs is still valid: freemem(fs) + below releases it. Same order as the_end() and the rebuild + paths (ASan caught the opposite order here). */ + time(&t); + fsck_progress("###########\n" + "reiserfsck finished at %s" "###########\n", ctime(&t)); + close(fs->fs_journal_dev); free(fs->fs_file_name); fs->fs_file_name = NULL; close(fs->fs_dev); freemem(fs); - time(&t); - fsck_progress("###########\n" - "reiserfsck finished at %s" "###########\n", ctime(&t)); - exit(EXIT_FIXED); } diff --git a/tests/vm/RESULTS.md b/tests/vm/RESULTS.md index 3cf3d30..02f289d 100644 --- a/tests/vm/RESULTS.md +++ b/tests/vm/RESULTS.md @@ -145,15 +145,28 @@ Ferramentaria: Fedora 44 (host) com **clang 22.1.8**, Debian 12 na VM com 5. **heap-use-after-free real, encontrado por ASan com clang**: `reiserfs_close()` chamava `fsync(fs->fs_dev)` DEPOIS de - `reiserfs_free()`, que libera o `fs` — leitura pendente de 4 bytes em + `reiserfs_free()` liberar `fs` — leitura pendente de 4 bytes em memória libertada (o ASan apontou exatamente `reiserfslib.c:406/407`; o gcc nunca acusou porque o heap ainda "devolve" o valor velho). Corrigido movendo o `fsync` para antes do free — a escrita já está no dispositivo neste ponto (`reiserfs_flush()` despeja journal e buffers; os `free_buffers()` de `reiserfs_free()` apenas validam e libertam, sem - escrever). Revalidado com ASan+UBSan em `halt_on_error`: mkfs, check, - tune, debugreiserfs, resize 64→128 MiB e rollback completo — **zero - erros**; bateria completa no host gcc continua 12/12. + escrever). + +6. **segundo heap-use-after-free, também no caminho de rollback** + (`fsck/main.c:1313`, `fsck_rollback()`): a função fazia `freemem(fs)` e + depois imprimia o banner final via `fsck_progress(...)`, que + desreferencia `fs` (`fsck_data(fs)` etc.). Capturado por ASan em uma + reexecução explícita do `--rollback-fsck-changes` com `halt_on_error` + (a rodada inicial de smoke não exercitou esse ramo até o fim). + Corrigido: o banner é impresso antes da limpeza, na mesma ordem usada + por `the_end()` e pelos caminhos de rebuild. Revalidado: save, restore, + rejeição de magic inválida e resize sob ASan+UBSan com `halt_on_error=1` + — **zero erros**, saída funcional inalterada. + +Ambos os builds saneados (mkfs, check, tune, debugreiserfs, resize +64→128 MiB e rollback completo) passam com **zero erros**; a bateria +completa no host gcc continua 12/12. ### Não-bugs registrados