On Thu, Apr 26, 2007 at 02:50:18AM +0200, Markus Lude wrote:
> 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.
I searched for pcap_timeval under /usr/include, as pcap headers are
located there, but pcap_timeval is declared in a patch to the snort
sources (patch-src_snort_packet_header_h).
> 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?
After some hints from Nikns Siankin new diff attached with pcap_timeval
instead of bpf_timeval. Diff works for me on sparc64 running -current.
Please test, comment and maybe commit.
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 Sun Apr 29 20:46:29 2007
@@ -0,0 +1,39 @@
+$OpenBSD$
+--- src/log.c.orig Wed Sep 6 23:04:25 2006
++++ src/log.c Sun Apr 29 20:44:21 2007
+@@ -347,7 +347,7 @@ void PrintIPPkt(FILE * fp, int type, Packet * p)
+ 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((struct pcap_timeval *) & p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, fp);
+@@ -758,7 +758,7 @@ void PrintArpHeader(FILE * fp, Packet * p)
+
+ 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((struct pcap_timeval *) & p->pkth->ts, timestamp);
+
+ /* determine what to use as MAC src and dst */
+ if (p->eh != NULL)
+@@ -1808,7 +1808,7 @@ void PrintEapolPkt(FILE * fp, Packet * p)
+
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print((struct pcap_timeval *) & p->pkth->ts, timestamp);
+
+ /* dump the timestamp */
+ fwrite(timestamp, strlen(timestamp), 1, fp);
+@@ -1982,7 +1982,7 @@ void PrintWifiPkt(FILE * fp, Packet * p)
+
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print((struct timeval *) & p->pkth->ts, timestamp);
++ ts_print((struct pcap_timeval *) & 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 Sun Apr
29 20:21:56 2007
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- src/output-plugins/spo_alert_fast.c.orig Fri Aug 12 22:22:14 2005
++++ src/output-plugins/spo_alert_fast.c Sun Apr 29 20:11:00 2007
+@@ -137,7 +137,7 @@ void AlertFast(Packet *p, char *msg, void *arg, Event
+ 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 : (struct pcap_timeval *) & 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 Sun Apr
29 20:21:58 2007
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- src/output-plugins/spo_alert_full.c.orig Fri Aug 12 22:22:14 2005
++++ src/output-plugins/spo_alert_full.c Sun Apr 29 20:11:09 2007
+@@ -161,7 +161,7 @@ void AlertFull(Packet *p, char *msg, void *arg, Event
+ 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 : (struct pcap_timeval *) & 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 Sun Apr 29
20:21:59 2007
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- src/output-plugins/spo_csv.c.orig Wed Jul 12 20:59:35 2006
++++ src/output-plugins/spo_csv.c Sun Apr 29 20:12:09 2007
+@@ -269,7 +269,7 @@ void RealAlertCSV(Packet * p, char *msg, FILE * file,
+ return;
+
+ bzero((char *) timestamp, TIMEBUF_SIZE);
+- ts_print(p == NULL ? NULL : (struct timeval *) & p->pkth->ts, timestamp);
++ ts_print(p == NULL ? NULL : (struct pcap_timeval *) & 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 Sun Apr
29 20:32:37 2007
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- src/preprocessors/spp_sfportscan.c.orig Wed May 24 18:06:55 2006
++++ src/preprocessors/spp_sfportscan.c Sun Apr 29 20:28:28 2007
+@@ -225,7 +225,7 @@ static int LogPortscanAlert(Packet *p, char *msg, u_in
+ return 0;
+ }
+
+- ts_print((struct timeval *)&p->pkth->ts, timebuf);
++ ts_print((struct pcap_timeval *)&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 Sun Apr 29 20:22:15 2007
@@ -0,0 +1,28 @@
+$OpenBSD$
+--- src/util.c.orig Wed Aug 2 17:55:49 2006
++++ src/util.c Sun Apr 29 20:14:27 2007
+@@ -333,12 +333,13 @@ int DisplayBanner()
+ * Returns: void function
+ *
+ ****************************************************************************/
+-void ts_print(register const struct timeval *tvp, char *timebuf)
++void ts_print(register const struct pcap_timeval *tvp, char *timebuf)
+ {
+ register int s;
+ int localzone;
+ time_t Time;
+ struct timeval tv;
++ struct pcap_timeval tvnow;
+ struct timezone tz;
+ struct tm *lt; /* place to stick the adjusted clock data */
+
+@@ -349,6 +350,9 @@ void ts_print(register const struct timeval *tvp, char
+ 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 Sun Apr 29 21:27:51 2007
@@ -0,0 +1,20 @@
+$OpenBSD$
+--- src/util.h.orig Fri Jun 30 20:17:42 2006
++++ src/util.h Sun Apr 29 21:27:16 2007
+@@ -41,6 +41,7 @@
+ #endif
+
+ #include "sfsnprintfappend.h"
++#include "snort_packet_header.h"
+
+ extern u_long netmasks[33];
+
+@@ -60,7 +61,7 @@ typedef struct _SPMemControl
+ int DisplayBanner();
+ void GetTime(char *);
+ int gmt2local(time_t);
+-void ts_print(register const struct timeval *, char *);
++void ts_print(register const struct pcap_timeval *, char *);
+ char *copy_argv(char **);
+ int strip(char *);
+ float CalcPct(float, float);
