Fix second use-after-free in the rollback path; clang battery re-validated
build / build (clang, ) (push) Failing after 6s
build / build (clang, --enable-warnings) (push) Failing after 6s
build / build (gcc, ) (push) Failing after 6s
build / build (gcc, --enable-io-failure-emulation) (push) Failing after 6s
build / build (gcc, --enable-warnings) (push) Failing after 7s
build / out-of-tree (push) Failing after 6s
build / sanitizers (push) Failing after 5s
build / build (clang, ) (push) Failing after 6s
build / build (clang, --enable-warnings) (push) Failing after 6s
build / build (gcc, ) (push) Failing after 6s
build / build (gcc, --enable-io-failure-emulation) (push) Failing after 6s
build / build (gcc, --enable-warnings) (push) Failing after 7s
build / out-of-tree (push) Failing after 6s
build / sanitizers (push) Failing after 5s
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).
This commit is contained in:
1 parent
7b5421ce49
commit
e760d19017
2 files changed
+24
-8
No files matched your search
+7
-4
@@ -1303,16 +1303,19 @@ static void fsck_rollback(reiserfs_filsys_t fs)
|
|||||||
fsck_progress_file(fs));
|
fsck_progress_file(fs));
|
||||||
close_rollback_file();
|
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);
|
close(fs->fs_journal_dev);
|
||||||
free(fs->fs_file_name);
|
free(fs->fs_file_name);
|
||||||
fs->fs_file_name = NULL;
|
fs->fs_file_name = NULL;
|
||||||
close(fs->fs_dev);
|
close(fs->fs_dev);
|
||||||
freemem(fs);
|
freemem(fs);
|
||||||
|
|
||||||
time(&t);
|
|
||||||
fsck_progress("###########\n"
|
|
||||||
"reiserfsck finished at %s" "###########\n", ctime(&t));
|
|
||||||
|
|
||||||
exit(EXIT_FIXED);
|
exit(EXIT_FIXED);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+17
-4
@@ -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**:
|
5. **heap-use-after-free real, encontrado por ASan com clang**:
|
||||||
`reiserfs_close()` chamava `fsync(fs->fs_dev)` DEPOIS de
|
`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
|
memória libertada (o ASan apontou exatamente `reiserfslib.c:406/407`; o
|
||||||
gcc nunca acusou porque o heap ainda "devolve" o valor velho). Corrigido
|
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
|
movendo o `fsync` para antes do free — a escrita já está no dispositivo
|
||||||
neste ponto (`reiserfs_flush()` despeja journal e buffers; os
|
neste ponto (`reiserfs_flush()` despeja journal e buffers; os
|
||||||
`free_buffers()` de `reiserfs_free()` apenas validam e libertam, sem
|
`free_buffers()` de `reiserfs_free()` apenas validam e libertam, sem
|
||||||
escrever). Revalidado com ASan+UBSan em `halt_on_error`: mkfs, check,
|
escrever).
|
||||||
tune, debugreiserfs, resize 64→128 MiB e rollback completo — **zero
|
|
||||||
erros**; bateria completa no host gcc continua 12/12.
|
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
|
### Não-bugs registrados
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user