tcpdump mailing list archives
Re: clean exit from tcpdump with asan
From: enh via tcpdump-workers <tcpdump-workers () lists tcpdump org>
Date: Mon, 19 Oct 2020 15:30:32 -0700
--- Begin Message --- From: enh <enh () google com>
Date: Mon, 19 Oct 2020 15:30:32 -0700
this patch fixes the use-after-free of `pd`, and also fixes the leak of `device`. let me know if you need this uploaded to github instead... diff --git a/tcpdump.c b/tcpdump.c index 658d8b34..4fa390fd 100644 --- a/tcpdump.c +++ b/tcpdump.c @@ -2239,8 +2239,12 @@ main(int argc, char **argv) error("%s", pcap_geterr(pd)); if (dflag) { bpf_dump(&fcode, dflag); - pcap_close(pd); + /* Clear pd so our signal handler won't use-after-free it. */ + pcap_t *to_free = pd; + pd = NULL; + pcap_close(to_free); free(cmdbuf); + free(device); pcap_freecode(&fcode); exit_tcpdump(S_SUCCESS); } @@ -2559,7 +2563,10 @@ DIAG_ON_CLANG(assign-enum) */ info(1); } - pcap_close(pd); + /* Clear pd so our signal handler won't use-after-free it. */ + pcap_t *to_free = pd; + pd = NULL; + pcap_close(to_free); if (VFileName != NULL) { ret = get_next_file(VFile, VFileLine); if (ret) { @@ -2640,6 +2647,7 @@ DIAG_ON_CLANG(assign-enum) PLURAL_SUFFIX(packets_captured)); free(cmdbuf); + free(device); pcap_freecode(&fcode); exit_tcpdump(status == -1 ? 1 : 0); } @@ -2692,7 +2700,8 @@ cleanup(int signo _U_) * to do anything with standard I/O streams in a signal handler - * the ANSI C standard doesn't say it is). */ - pcap_breakloop(pd); + if (pd != NULL) + pcap_breakloop(pd); #else /* * We don't have "pcap_breakloop()"; this isn't safe, but On Wed, Oct 14, 2020 at 2:28 PM enh <enh () google com> wrote:i haven't reproduced it myself yet (though i'll try shortly) but we got an automated crash report from tcpdump on Android via [gwp-asan](https://developer.android.com/ndk/guides/gwp-asan). the bug is a use-after-free, specifically when pcap_breakloop() tries to write to the already-freed struct pcap_t. this happens if a signal is received during tcpdump shutdown (which is presumably why we haven't hit this more often on ASan/HWASan builds). i assume the fix is to disable the signal handlers before calling pcap_close() to free the struct pcap_t, but i thought i'd bring this up on the list before i (a) look at reproducing this locally and (b) send a patch...Attachment: tcpdump-hwasan.txt
Description:
--- End Message ---
_______________________________________________ tcpdump-workers mailing list tcpdump-workers () lists tcpdump org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Current thread:
- clean exit from tcpdump with asan enh via tcpdump-workers (Oct 14)
- <Possible follow-ups>
- Re: clean exit from tcpdump with asan enh via tcpdump-workers (Oct 19)
- Message not available
- Re: clean exit from tcpdump with asan enh via tcpdump-workers (Oct 27)
- Message not available