On Sat, Apr 07, 2007 at 03:42:25PM -0600, Peter Valchev wrote:
> > > Ok, I think I've found the root cause of the problem, but it's strange.
> > > In ts_print() function in util.c (just for those interested in locating
> > > the line below in snort source code), there is a line like this:
> > >
> > > s = (tvp->tv_sec + localzone) % 86400;
> > >
> > > On amd64, this produces random output (not so random, but anyway), which
> > > is the value of s. To fix this problem, all I have to do is to expand
> > > that line as follows:
> > >
> > > temp = tvp->tv_sec + localzone;
> > > s = temp % 86400;
> > >
> > > Since I felt that this is a very dumb thing to do (and just a work
> > > around), I suspected the type of s. So I used different types for s, but
> > > nothing has changed. Also, since I tried -O0 compiler option, I don't
> > > think it's an optimization problem.
> > >
> > > Could somebody explain how this is possible? On amd64 what is it that's
> > > different from i386 and can cause a problem like this? And what is the
> > > correct way of fixing it?
> >
> > >From a quick look, it seems that it's a problem with struct timeval and
> > the type of tv_sec. The one in sys/time.h is long (64-bit in that case),
> > and the one from pcap is defined as a 32-bit int. Mixing the two makes
> > for strange things like you're seeing...
>
> Just to reiterate, snort/its plugins call ts_print() with an argument
> pcap_timeval, which has 32-bit tv_sec (uint32_t). ts_print however,
> works with sys/time.h timeval, which has 64-bit tv_sec (long). On i386
> this is no problem of course.
>
> One has to carefully trace the actual problem (I have not done that yet)
> and make the usages consistent.
bpf_timeval is used somewhat nested in snort's struct Packet. I couldn't
find any reference of pcap_timeval. But tv_sec in bpf_timeval is
u_int32_t versus long in timeval.
As mentioned, this is a problem on 64bit architectures like amd64 and
here on sparc64, too. A diff is attached which fixes this for me on
sparc64. Could someone please try it on some 32bit architecture?
I removed the casts in ts_print() calls and tried to fix the use of the
changed parameter type in ts_print(). I'll send this upstream if noone
has a problem with this during the next days/weeks.
As this is my first patch I hope the format is not too much confusing.
How do you make a diff against a current port?
Please test and comment.
Regards,
Markus
diff -ruNP snort/patches/patch-src_log_c
../mystuff/snort/patches/patch-src_log_c
--- snort/patches/patch-src_log_c Thu Jan 1 01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_log_c Thu Apr 26 02:21:20 2007
@@ -0,0 +1,38 @@
+--- src/log.c.orig Wed Sep 6 23:04:25 2006
++++ src/log.c Wed Apr 25 22:43:47 2007
+@@ -347,7 +347,7 @@
+ DEBUG_WRAP(DebugMessage(DEBUG_LOG, "PrintIPPkt type = %d\n", type););
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(& p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, fp);
+@@ -758,7 +758,7 @@
+
+ bzero((struct in_addr *) &ip_addr, sizeof(struct in_addr));
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(& p->pkth->ts, timestamp);
+
+ /* determine what to use as MAC src and dst */
+ if (p->eh != NULL)
+@@ -1808,7 +1808,7 @@
+
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(& p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, fp);
+@@ -1982,7 +1982,7 @@
+
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(& p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, fp);
diff -ruNP snort/patches/patch-src_output-plugins_spo_alert_fast_c
../mystuff/snort/patches/patch-src_output-plugins_spo_alert_fast_c
--- snort/patches/patch-src_output-plugins_spo_alert_fast_c Thu Jan 1
01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_output-plugins_spo_alert_fast_c Thu Apr
26 02:21:20 2007
@@ -0,0 +1,11 @@
+--- src/output-plugins/spo_alert_fast.c.orig Fri Aug 12 22:22:14 2005
++++ src/output-plugins/spo_alert_fast.c Wed Apr 25 23:18:12 2007
+@@ -137,7 +137,7 @@
+ SpoAlertFastData *data = (SpoAlertFastData *)arg;
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print(p == NULL ? NULL : (struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(p == NULL ? NULL : & p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, data->file);
diff -ruNP snort/patches/patch-src_output-plugins_spo_alert_full_c
../mystuff/snort/patches/patch-src_output-plugins_spo_alert_full_c
--- snort/patches/patch-src_output-plugins_spo_alert_full_c Thu Jan 1
01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_output-plugins_spo_alert_full_c Thu Apr
26 02:21:20 2007
@@ -0,0 +1,11 @@
+--- src/output-plugins/spo_alert_full.c.orig Fri Aug 12 22:22:14 2005
++++ src/output-plugins/spo_alert_full.c Wed Apr 25 23:18:49 2007
+@@ -161,7 +161,7 @@
+ DEBUG_WRAP(DebugMessage(DEBUG_LOG, "Logging Alert data!\n"););
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print(p == NULL ? NULL : (struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(p == NULL ? NULL : & p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, data->file);
diff -ruNP snort/patches/patch-src_output-plugins_spo_csv_c
../mystuff/snort/patches/patch-src_output-plugins_spo_csv_c
--- snort/patches/patch-src_output-plugins_spo_csv_c Thu Jan 1 01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_output-plugins_spo_csv_c Thu Apr 26
02:21:20 2007
@@ -0,0 +1,11 @@
+--- src/output-plugins/spo_csv.c.orig Wed Jul 12 20:59:35 2006
++++ src/output-plugins/spo_csv.c Wed Apr 25 23:17:12 2007
+@@ -269,7 +269,7 @@
+ return;
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print(p == NULL ? NULL : (struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(p == NULL ? NULL : & p->pkth->ts, timestamp);
+
+ DEBUG_WRAP(DebugMessage(DEBUG_LOG,"Logging CSV Alert data\n"););
+
diff -ruNP snort/patches/patch-src_preprocessors_spp_sfportscan_c
../mystuff/snort/patches/patch-src_preprocessors_spp_sfportscan_c
--- snort/patches/patch-src_preprocessors_spp_sfportscan_c Thu Jan 1
01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_preprocessors_spp_sfportscan_c Thu Apr
26 02:21:20 2007
@@ -0,0 +1,11 @@
+--- src/preprocessors/spp_sfportscan.c.orig Wed May 24 18:06:55 2006
++++ src/preprocessors/spp_sfportscan.c Wed Apr 25 22:45:31 2007
+@@ -225,7 +225,7 @@
+ return 0;
+ }
+
+- ts_print((struct timeval *)&p->pkth->ts, timebuf);
++ ts_print(&p->pkth->ts, timebuf);
+
+ fprintf(g_logfile, "Time: %s\n", timebuf);
+
diff -ruNP snort/patches/patch-src_util_c
../mystuff/snort/patches/patch-src_util_c
--- snort/patches/patch-src_util_c Thu Jan 1 01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_util_c Thu Apr 26 02:21:20 2007
@@ -0,0 +1,28 @@
+--- src/util.c.orig Wed Aug 2 17:55:49 2006
++++ src/util.c Thu Apr 26 00:39:42 2007
+@@ -333,12 +333,13 @@
+ * Returns: void function
+ *
+ ****************************************************************************/
+-void ts_print(register const struct timeval *tvp, char *timebuf)
++void ts_print(register const struct bpf_timeval *tvp, char *timebuf)
+ {
+ register int s;
+ int localzone;
+ time_t Time;
+ struct timeval tv;
++ struct bpf_timeval tvnow;
+ struct timezone tz;
+ struct tm *lt; /* place to stick the adjusted clock data */
+
+@@ -348,7 +349,9 @@
+ /* manual page (for linux) says tz is never used, so.. */
+ bzero((char *) &tz, sizeof(tz));
+ gettimeofday(&tv, &tz);
+- tvp = &tv;
++ tvnow.tv_sec = tv.tv_sec;
++ tvnow.tv_usec = tv.tv_usec;
++ tvp = &tvnow;
+ }
+
+ localzone = thiszone;
diff -ruNP snort/patches/patch-src_util_h
../mystuff/snort/patches/patch-src_util_h
--- snort/patches/patch-src_util_h Thu Jan 1 01:00:00 1970
+++ ../mystuff/snort/patches/patch-src_util_h Thu Apr 26 02:21:20 2007
@@ -0,0 +1,19 @@
+--- src/util.h.orig Fri Jun 30 20:17:42 2006
++++ src/util.h Thu Apr 26 00:47:13 2007
+@@ -26,6 +26,7 @@
+ #ifndef WIN32
+ #include <sys/time.h>
+ #include <sys/types.h>
++#include <net/bpf.h>
+ #endif /* !WIN32 */
+
+ #ifdef HAVE_CONFIG_H
+@@ -60,7 +61,7 @@
+ int DisplayBanner();
+ void GetTime(char *);
+ int gmt2local(time_t);
+-void ts_print(register const struct timeval *, char *);
++void ts_print(register const struct bpf_timeval *, char *);
+ char *copy_argv(char **);
+ int strip(char *);
+ float CalcPct(float, float);
