[tcpdump-workers] clean exit from tcpdump with asan
--- Begin Message --- 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... --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] clean exit from tcpdump with asan
--- Begin Message ---
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 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...
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
--- End Message ---
___
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] clean exit from tcpdump with asan
--- Begin Message --- On Tue, Oct 27, 2020 at 5:42 AM Michael Richardson wrote: > > > enh via tcpdump-workers wrote: > > 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... > > Please make a pull request. > That enables all sorts of cross-builds. done: https://github.com/the-tcpdump-group/tcpdump/pull/888 > I am thinking about how we might test signals during processing in a > deterministic way. yeah, the crash dump i have is from someone's test device. personally i added sleep()s to be able to win races manually, but i don't know how to unit test this. > -- > ] Never tell me the odds! | ipv6 mesh networks [ > ] Michael Richardson, Sandelman Software Works|IoT architect [ > ] m...@sandelman.ca http://www.sandelman.ca/| ruby on rails > [ > --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] So which is cooler - tcpdump on your wrist or tcpdump on your Mac's Touch Bar?
--- Begin Message ---
i don't know about iOS, but on Android tcpdump is only on engineering
builds ("userdebug" and "eng" builds, for those familiar with such
things), not shipped on production devices ("user builds").
(at least not by default. an OEM could change that, but they tend to lean
in the opposite direction.)
On Tue, Jan 5, 2021 at 5:58 PM Guy Harris via tcpdump-workers <
tcpdump-workers@lists.tcpdump.org> wrote:
>
>
>
> -- Forwarded message --
> From: Guy Harris
> To: tcpdump-workers
> Cc:
> Bcc:
> Date: Tue, 5 Jan 2021 17:58:14 -0800
> Subject: So which is cooler - tcpdump on your wrist or tcpdump on your
> Mac's Touch Bar?
> $ curl -s
> https://opensource.apple.com/source/tcpdump/tcpdump-100/tcpdump.xcodeproj/project.pbxproj.auto.html
> | egrep SUPPORTED_PLATFORMS
> SUPPORTED_PLATFORMS = "macosx
> iphoneos appletvos watchos bridgeos";
> SUPPORTED_PLATFORMS = "macosx
> iphoneos appletvos watchos bridgeos";
>
> (bridgeOS is the apparently-watchOS-derived OS that runs on the T-series
> chips that run the Touch Bar on Touch Bar Macs and that handle secure
> booting and possibly some other security stuff.)
>
> I don't know whether it ships on iOS/iPadOS/tvOS/watchOS/bridgeOS or is
> just built to be used in-house.
>
>
> -- Forwarded message --
> From: Guy Harris via tcpdump-workers
> To: tcpdump-workers
> Cc:
> Bcc:
> Date: Tue, 5 Jan 2021 17:58:14 -0800
> Subject: [tcpdump-workers] So which is cooler - tcpdump on your wrist or
> tcpdump on your Mac's Touch Bar?
> ___
> tcpdump-workers mailing list
> tcpdump-workers@lists.tcpdump.org
> https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
--- End Message ---
___
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
