Package: tcptrace
Version: 6.6.7-3
Severity: normal
While tracking down a memory corruption bug in mod_http.c, valgrind reported
invalid memory use in tcpdump.c. I think that this is because libpcap
stack-allocates the packet header data whose address it passes to the callback
function; the callback function in tcpdump.c saves the pointer for later use.
This "works" (as in mostly doesn't break) because tcpdump.c immediately uses
the pointer and then discards it, but it's not correct and it causes valgrind
to whine.
I know that pacifying valgrind for its own sake can be dangerous :-/ but I
think that the attached change is correct.
-- System Information:
Debian Release: 6.0.1
APT prefers stable
APT policy: (900, 'stable')
Architecture: i386 (i686)
Kernel: Linux 2.6.37+ (SMP w/1 CPU core)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Versions of packages tcptrace depends on:
ii libc6 2.11.2-10 Embedded GNU C Library: Shared lib
ii libpcap0.8 1.1.1-2 system interface for user-level pa
Versions of packages tcptrace recommends:
ii tcpdump 4.1.1-2csr1 command-line network traffic analy
ii xplot-xplot.org 0.90.7.1-2 fast tool to graph and visualize l
tcptrace suggests no packages.
-- no debconf information
diff -uwr tcptrace-6.6.7/tcpdump.c tcptrace-6.6.7-mine//tcpdump.c
--- tcptrace-6.6.7/tcpdump.c 2011-04-26 21:11:05.000000000 +0100
+++ tcptrace-6.6.7-mine//tcpdump.c 2011-04-26 21:08:41.000000000 +0100
@@ -75,7 +75,7 @@
static struct ether_header eth_header;
#define EH_SIZE sizeof(struct ether_header)
static char *ip_buf; /* [IP_MAXPACKET] */
-static struct pcap_pkthdr *callback_phdr;
+static struct pcap_pkthdr callback_phdr;
static void *callback_plast;
@@ -100,7 +100,7 @@
type = pcap_datalink(pcap);
/* remember the stuff we always save */
- callback_phdr = phdr;
+ memmove(&callback_phdr, phdr, sizeof(struct pcap_pkthdr));
/* kindof ugly, but about the only way to make them fit together :-( */
switch (type) {
@@ -264,14 +264,14 @@
/* This confuses EVERYTHING. Try to compensate. */
{
static Bool bogus_nanoseconds = FALSE;
- if ((callback_phdr->ts.tv_usec >= US_PER_SEC) ||
+ if ((callback_phdr.ts.tv_usec >= US_PER_SEC) ||
(bogus_nanoseconds)) {
if (!bogus_nanoseconds) {
fprintf(stderr,
"tcpdump: attempting to adapt to bogus nanosecond timestamps\n");
bogus_nanoseconds = TRUE;
}
- callback_phdr->ts.tv_usec /= 1000;
+ callback_phdr.ts.tv_usec /= 1000;
}
}
@@ -281,10 +281,10 @@
*ppip = (struct ip *) ip_buf;
*pplast = callback_plast; /* last byte in IP packet */
/* (copying time structure in 2 steps to avoid RedHat brain damage) */
- ptime->tv_usec = callback_phdr->ts.tv_usec;
- ptime->tv_sec = callback_phdr->ts.tv_sec;
- *plen = callback_phdr->len;
- *ptlen = callback_phdr->caplen;
+ ptime->tv_usec = callback_phdr.ts.tv_usec;
+ ptime->tv_sec = callback_phdr.ts.tv_sec;
+ *plen = callback_phdr.len;
+ *ptlen = callback_phdr.caplen;
/* if it's not IP, then skip it */
if ((ntohs(eth_header.ether_type) != ETHERTYPE_IP) &&
