NetBSD Problem Report #47101
From tn@catvmics.ne.jp Sun Oct 21 22:00:29 2012
Return-Path: <tn@catvmics.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
by www.NetBSD.org (Postfix) with ESMTP id 0F4EB63E042
for <gnats-bugs@gnats.NetBSD.org>; Sun, 21 Oct 2012 22:00:29 +0000 (UTC)
Message-Id: <20121021201345.45015A5F3@gw.iv.dyndns.org>
Date: Mon, 22 Oct 2012 05:13:45 +0900 (JST)
From: nakayama@netbsd.org
Reply-To: nakayama@netbsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: ipmon(8) alignment problem on 32-bit sparc
X-Send-Pr-Version: 3.95
>Number: 47101
>Category: bin
>Synopsis: ipmon(8) alignment problem on 32-bit sparc
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: nakayama
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sun Oct 21 22:05:00 +0000 2012
>Closed-Date: Mon Nov 19 19:11:17 +0000 2012
>Last-Modified: Mon Nov 19 19:11:17 +0000 2012
>Originator: Takeshi Nakayama
>Release: NetBSD 6.0
>Organization:
>Environment:
System: NetBSD nyx 6.0 NetBSD 6.0 (NYX32) #0: Mon Oct 15 08:57:59 JST 2012
takeshi@nyx:/usr/src/sys/arch/sparc64/compile/NYX32 sparc64
Architecture: sparc
Machine: sparc64
>Description:
ipmon(8) sometimes dies with bus error on 32-bit sparc. Since 6.0,
ipf log records contain time_t data and read it with "ldd"
instruction, but it requires 8-byte alignment.
nyx# gdb ipmon /ipmon.core
GNU gdb (GDB) 7.3.1
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "sparc--netbsdelf".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /w1/obj/6-sparc64_32/usr.sbin/ipf/ipmon/ipmon...(no debugging symbols found)...done.
[New process 1]
Core was generated by `ipmon'.
Program terminated with signal 10, Bus error.
#0 0x00012430 in print_log ()
(gdb) where
#0 0x00012430 in print_log ()
#1 0x000145b8 in main ()
(gdb) disas
Dump of assembler code for function print_log:
0x00011e20 <+0>: save %sp, -232, %sp
0x00011e24 <+4>: sethi %hi(0x2b000), %l0
0x00011e28 <+8>: cmp %i3, 0
(snip)
0x00012424 <+1540>: clrb [ %i5 + 0x2c8 ]
0x00012428 <+1544>: add %i2, 0x6c, %g4
0x0001242c <+1548>: or %i4, 0x2a0, %o5
=> 0x00012430 <+1552>: ldd [ %i2 + 8 ], %g2
0x00012434 <+1556>: srl %g1, 4, %g1
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) p/x $i2
$1 = 0xffffa9c4
(gdb) p/x $g4
$2 = 0xffffaa30
(gdb) quit
>How-To-Repeat:
Run ipmon(8) on 32-bit sparc, NetBSD 6.0. I guess -current has the
same problem.
>Fix:
I'm not sure ALIGNBYTES is suitable for all ports, but it's fine at
least on 32-bit saprc.
--- dist/ipf/tools/ipmon.c.orig 2012-02-16 11:46:21.000000000 +0900
+++ dist/ipf/tools/ipmon.c 2012-10-16 11:34:26.000000000 +0900
@@ -946,7 +946,7 @@
while (blen > 0) {
ipl = (iplog_t *)buf;
- if ((u_long)ipl & (sizeof(long)-1)) {
+ if ((u_long)ipl & ALIGNBYTES) {
if (bp)
bpo = bp;
bp = (char *)malloc(blen);
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: bin-bug-people->darrenr
Responsible-Changed-By: martin@NetBSD.org
Responsible-Changed-When: Tue, 23 Oct 2012 11:05:29 +0000
Responsible-Changed-Why:
Over to maintainer
From: jnemeth@victoria.tc.ca (John Nemeth)
To: gnats-bugs@NetBSD.org, darrenr@NetBSD.org, gnats-admin@NetBSD.org,
netbsd-bugs@NetBSD.org, martin@NetBSD.org, nakayama@NetBSD.org
Cc:
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Tue, 23 Oct 2012 08:04:30 -0700
On Feb 7, 11:12pm, martin@NetBSD.org wrote:
}
} Synopsis: ipmon(8) alignment problem on 32-bit sparc
}
} Responsible-Changed-From-To: bin-bug-people->darrenr
} Responsible-Changed-By: martin@NetBSD.org
} Responsible-Changed-When: Tue, 23 Oct 2012 11:05:29 +0000
} Responsible-Changed-Why:
} Over to maintainer
Didn't Christos fix this? Although the path is different in
-current, so it might need a pullup.
}-- End of excerpt from martin@NetBSD.org
From: christos@zoulas.com (Christos Zoulas)
To: jnemeth@victoria.tc.ca (John Nemeth), gnats-bugs@NetBSD.org,
darrenr@NetBSD.org, gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org,
martin@NetBSD.org, nakayama@NetBSD.org
Cc:
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Tue, 23 Oct 2012 11:26:15 -0400
On Oct 23, 8:04am, jnemeth@victoria.tc.ca (John Nemeth) wrote:
-- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
| Didn't Christos fix this? Although the path is different in
| -current, so it might need a pullup.
Yes, I fixed it.
christos
From: Takeshi Nakayama <nakayama@NetBSD.org>
To: christos@zoulas.com
Cc: jnemeth@victoria.tc.ca, gnats-bugs@NetBSD.org, darrenr@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, martin@NetBSD.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Thu, 25 Oct 2012 22:43:28 +0900 (JST)
----Next_Part(Thu_Oct_25_22_43_28_2012_891)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
>>> christos@zoulas.com (Christos Zoulas) wrote
> On Oct 23, 8:04am, jnemeth@victoria.tc.ca (John Nemeth) wrote:
> -- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
>
> | Didn't Christos fix this? Although the path is different in
> | -current, so it might need a pullup.
>
> Yes, I fixed it.
Thanks.
I backported your fixes[*] to the netbsd-6 branch as attached. If
there are no objections, I will send a pullup request.
[*] http://mail-index.netbsd.org/source-changes/2012/10/21/msg038121.html
http://mail-index.netbsd.org/source-changes/2012/10/22/msg038125.html
http://mail-index.netbsd.org/source-changes/2012/10/22/msg038126.html
-- Takeshi Nakayama
----Next_Part(Thu_Oct_25_22_43_28_2012_891)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="ipmon-fix-nb6.diff"
Index: src/dist/ipf/ipmon.h
===================================================================
RCS file: /cvsroot/src/dist/ipf/Attic/ipmon.h,v
retrieving revision 1.2
diff -u -d -r1.2 ipmon.h
--- src/dist/ipf/ipmon.h 15 Feb 2012 17:55:04 -0000 1.2
+++ src/dist/ipf/ipmon.h 25 Oct 2012 02:08:37 -0000
@@ -84,14 +84,14 @@
#define OPT_PORTNUM 0x400
#define OPT_LOGALL (OPT_NAT|OPT_STATE|OPT_FILTER)
-#define HOSTNAME_V4(a,b) hostname((a), 4, (u_32_t *)&(b))
+#define HOSTNAME_V4(a,b) hostname((a), 4, (const void *)&(b))
#ifndef LOGFAC
#define LOGFAC LOG_LOCAL0
#endif
extern int load_config(char *);
-extern void dumphex(FILE *, int, char *, int);
-extern int check_action(char *, char *, int, int);
+extern void dumphex(FILE *, int, const void *, size_t);
+extern int check_action(const char *, const char *, int, int);
extern char *getword(int);
extern int fac_findname(char *);
Index: src/dist/ipf/tools/ipmon.c
===================================================================
RCS file: /cvsroot/src/dist/ipf/tools/Attic/ipmon.c,v
retrieving revision 1.21
diff -u -d -r1.21 ipmon.c
--- src/dist/ipf/tools/ipmon.c 15 Feb 2012 17:55:11 -0000 1.21
+++ src/dist/ipf/tools/ipmon.c 25 Oct 2012 02:08:37 -0000
@@ -140,32 +140,31 @@
static FILE *binarylog = NULL;
static char *binarylogfile = NULL;
static int donehup = 0;
-static void usage __P((char *));
-static void handlehup __P((int));
-static void flushlogs __P((char *, FILE *));
-static void print_log __P((int, FILE *, char *, int));
-static void print_ipflog __P((FILE *, char *, int));
-static void print_natlog __P((FILE *, char *, int));
-static void print_statelog __P((FILE *, char *, int));
-static int read_log __P((int, int *, char *, int));
-static void write_pid __P((char *));
-static char *icmpname __P((u_int, u_int));
-static char *icmpname6 __P((u_int, u_int));
-static icmp_type_t *find_icmptype __P((int, icmp_type_t *, size_t));
-static icmp_subtype_t *find_icmpsubtype __P((int, icmp_subtype_t *, size_t));
+static void usage(const char *);
+static void handlehup(int);
+static void flushlogs(const char *, FILE *);
+static void print_log(int, FILE *, const void *, size_t);
+static void print_ipflog(FILE *, const void *, size_t);
+static void print_natlog(FILE *, const void *, size_t);
+static void print_statelog(FILE *, const void *, size_t);
+static int read_log(int, size_t *, void *, size_t);
+static void write_pid(const char *);
+static char *icmpname(u_int, u_int);
+static char *icmpname6(u_int, u_int);
+static icmp_type_t *find_icmptype(int, icmp_type_t *, size_t);
+static icmp_subtype_t *find_icmpsubtype(int, icmp_subtype_t *, size_t);
#ifdef __hpux
-static struct tm *get_tm __P((u_32_t));
+static struct tm *get_tm(u_32_t);
#else
-static struct tm *get_tm __P((time_t));
+static struct tm *get_tm(time_t);
#endif
-char *hostname __P((int, int, u_32_t *));
-char *portname __P((int, char *, u_int));
-int main __P((int, char *[]));
+char *hostname(int, int, const void *);
+char *portname(int, char *, u_int);
-static void logopts __P((int, char *));
-static void init_tabs __P((void));
-static char *getproto __P((u_int));
+static void logopts(int, const char *);
+static void init_tabs(void);
+static char *getproto(u_int);
static char **protocols = NULL;
static char **udp_ports = NULL;
@@ -186,7 +185,7 @@
#define OPT_LOGALL (OPT_NAT|OPT_STATE|OPT_FILTER)
#define OPT_LOGBODY 0x800
-#define HOSTNAME_V4(a,b) hostname((a), 4, (u_32_t *)&(b))
+#define HOSTNAME_V4(a,b) hostname((a), 4, (const void *)&(b))
#ifndef LOGFAC
#define LOGFAC LOG_LOCAL0
@@ -337,10 +336,8 @@
{ -2, NULL, 0, NULL }
};
-static icmp_subtype_t *find_icmpsubtype(type, table, tablesz)
-int type;
-icmp_subtype_t *table;
-size_t tablesz;
+static icmp_subtype_t *
+find_icmpsubtype(int type, icmp_subtype_t *table, size_t tablesz)
{
icmp_subtype_t *ist;
int i;
@@ -362,10 +359,8 @@
}
-static icmp_type_t *find_icmptype(type, table, tablesz)
-int type;
-icmp_type_t *table;
-size_t tablesz;
+static icmp_type_t *
+find_icmptype(int type, icmp_type_t *table, size_t tablesz)
{
icmp_type_t *it;
int i;
@@ -387,15 +382,16 @@
}
-static void handlehup(sig)
-int sig;
+static void
+handlehup(int sig)
{
signal(SIGHUP, handlehup);
donehup = 1;
}
-static void init_tabs()
+static void
+init_tabs(void)
{
struct protoent *p;
struct servent *s;
@@ -480,8 +476,8 @@
}
-static char *getproto(p)
-u_int p;
+static char *
+getproto(u_int p)
{
static char pnum[4];
char *s;
@@ -496,11 +492,10 @@
}
-static int read_log(fd, lenp, buf, bufsize)
-int fd, bufsize, *lenp;
-char *buf;
+static int
+read_log(int fd, size_t *lenp, void *buf, size_t bufsize)
{
- int nr;
+ ssize_t nr;
nr = read(fd, buf, bufsize);
if (!nr)
@@ -512,9 +507,8 @@
}
-char *hostname(res, v, ip)
-int res, v;
-u_32_t *ip;
+char *
+hostname(int res, int v, const void *ip)
{
# define MAX_INETA 16
static char hname[MAXHOSTNAMELEN + MAX_INETA + 3];
@@ -525,10 +519,10 @@
struct in_addr ipa;
if (v == 4) {
- ipa.s_addr = *ip;
+ ipa.s_addr = *(const u_32_t *)ip;
if (!res)
return inet_ntoa(ipa);
- hp = gethostbyaddr((char *)ip, sizeof(*ip), AF_INET);
+ hp = gethostbyaddr(ip, 4, AF_INET);
if (!hp)
return inet_ntoa(ipa);
sprintf(hname, "%.*s[%s]", MAXHOSTNAMELEN, hp->h_name,
@@ -545,10 +539,8 @@
}
-char *portname(res, proto, port)
-int res;
-char *proto;
-u_int port;
+char *
+portname(int res, char *proto, u_int port)
{
static char pname[8];
char *s;
@@ -569,9 +561,8 @@
}
-static char *icmpname(type, code)
-u_int type;
-u_int code;
+static char *
+icmpname(u_int type, u_int code)
{
static char name[80];
icmp_subtype_t *ist;
@@ -600,9 +591,8 @@
return name;
}
-static char *icmpname6(type, code)
-u_int type;
-u_int code;
+static char *
+icmpname6(u_int type, u_int code)
{
static char name[80];
icmp_subtype_t *ist;
@@ -632,11 +622,8 @@
}
-void dumphex(log, dopts, buf, len)
-FILE *log;
-int dopts;
-char *buf;
-int len;
+void
+dumphex(FILE *log, int dopts, const void *buf, size_t len)
{
char hline[80];
int i, j, k;
@@ -696,12 +683,14 @@
}
-static struct tm *get_tm(sec)
+static struct tm *
+get_tm(
#ifdef __hpux
-u_32_t sec;
+u_32_t sec
#else
-time_t sec;
+time_t sec
#endif
+)
{
struct tm *tm;
time_t t;
@@ -711,19 +700,17 @@
return tm;
}
-static void print_natlog(log, buf, blen)
-FILE *log;
-char *buf;
-int blen;
+static void
+print_natlog(FILE *log, const void *buf, size_t blen)
{
- struct natlog *nl;
- iplog_t *ipl = (iplog_t *)buf;
+ const struct natlog *nl;
+ const iplog_t *ipl = (const iplog_t *)buf;
char *t = line;
struct tm *tm;
int res, i, len;
char *proto;
- nl = (struct natlog *)((char *)ipl + sizeof(*ipl));
+ nl = (const struct natlog *)((const char *)ipl + sizeof(*ipl));
res = (opts & OPT_RESOLVE) ? 1 : 0;
tm = get_tm(ipl->ipl_sec);
len = sizeof(line);
@@ -795,18 +782,16 @@
}
-static void print_statelog(log, buf, blen)
-FILE *log;
-char *buf;
-int blen;
+static void
+print_statelog(FILE *log, const void *buf, size_t blen)
{
- struct ipslog *sl;
- iplog_t *ipl = (iplog_t *)buf;
+ const struct ipslog *sl;
+ const iplog_t *ipl = (const iplog_t *)buf;
char *t = line, *proto;
struct tm *tm;
int res, i, len;
- sl = (struct ipslog *)((char *)ipl + sizeof(*ipl));
+ sl = (const struct ipslog *)((const char *)ipl + sizeof(*ipl));
res = (opts & OPT_RESOLVE) ? 1 : 0;
tm = get_tm(ipl->ipl_sec);
len = sizeof(line);
@@ -935,65 +920,49 @@
}
-static void print_log(logtype, log, buf, blen)
-FILE *log;
-char *buf;
-int logtype, blen;
+static void
+print_log(int logtype, FILE *log, const void *buf, size_t blen)
{
- iplog_t *ipl;
- char *bp = NULL, *bpo = NULL;
+ iplog_t ipl;
int psize;
while (blen > 0) {
- ipl = (iplog_t *)buf;
- if ((u_long)ipl & (sizeof(long)-1)) {
- if (bp)
- bpo = bp;
- bp = (char *)malloc(blen);
- bcopy((char *)ipl, bp, blen);
- if (bpo) {
- free(bpo);
- bpo = NULL;
- }
- buf = bp;
- continue;
- }
+ if (sizeof(ipl) > blen)
+ return;
- psize = ipl->ipl_dsize;
+ memcpy(&ipl, buf, sizeof(ipl));
+ psize = ipl.ipl_dsize;
if (psize > blen)
- break;
+ return;
if (binarylog) {
fwrite(buf, psize, 1, binarylog);
fflush(binarylog);
}
- if (logtype == IPL_LOGIPF) {
- if (ipl->ipl_magic == IPL_MAGIC)
+ switch (logtype) {
+ case IPL_LOGIPF:
+ if (ipl.ipl_magic == IPL_MAGIC)
print_ipflog(log, buf, psize);
-
- } else if (logtype == IPL_LOGNAT) {
- if (ipl->ipl_magic == IPL_MAGIC_NAT)
+ break;
+ case IPL_LOGNAT:
+ if (ipl.ipl_magic == IPL_MAGIC_NAT)
print_natlog(log, buf, psize);
-
- } else if (logtype == IPL_LOGSTATE) {
- if (ipl->ipl_magic == IPL_MAGIC_STATE)
+ break;
+ case IPL_LOGSTATE:
+ if (ipl.ipl_magic == IPL_MAGIC_STATE)
print_statelog(log, buf, psize);
+ break;
}
blen -= psize;
- buf += psize;
+ buf = (const char *)buf + psize;
}
- if (bp)
- free(bp);
- return;
}
-static void print_ipflog(log, buf, blen)
-FILE *log;
-char *buf;
-int blen;
+static void
+print_ipflog(FILE *log, const void *buf, size_t blen)
{
tcphdr_t *tp;
struct icmp *ic;
@@ -1001,11 +970,11 @@
struct tm *tm;
char *t, *proto;
int i, v, lvl, res, len, off, plen, ipoff, defaction;
- ip_t *ipc, *ip;
+ const ip_t *ipc, *ip;
u_32_t *s, *d;
u_short hl, p;
- ipflog_t *ipf;
- iplog_t *ipl;
+ const ipflog_t *ipf;
+ const iplog_t *ipl;
#ifdef USE_INET6
struct ip6_ext *ehp;
u_short ehl;
@@ -1013,9 +982,9 @@
int go;
#endif
- ipl = (iplog_t *)buf;
- ipf = (ipflog_t *)((char *)buf + sizeof(*ipl));
- ip = (ip_t *)((char *)ipf + sizeof(*ipf));
+ ipl = (const iplog_t *)buf;
+ ipf = (const ipflog_t *)((const char *)buf + sizeof(*ipl));
+ ip = (const ip_t *)((const char *)ipf + sizeof(*ipf));
v = IP_V(ip);
res = (opts & OPT_RESOLVE) ? 1 : 0;
t = line;
@@ -1379,16 +1348,16 @@
}
-static void usage(prog)
-char *prog;
+static void
+usage(const char *prog)
{
fprintf(stderr, "%s: [-NFhstvxX] [-f <logfile>]\n", prog);
exit(1);
}
-static void write_pid(file)
-char *file;
+static void
+write_pid(const char *file)
{
FILE *fp = NULL;
int fd;
@@ -1407,9 +1376,8 @@
}
-static void flushlogs(file, log)
-char *file;
-FILE *log;
+static void
+flushlogs(const char *file, FILE *log)
{
int fd, flushed = 0;
@@ -1438,12 +1406,11 @@
}
-static void logopts(turnon, options)
-int turnon;
-char *options;
+static void
+logopts(int turnon, const char *options)
{
int flags = 0;
- char *s;
+ const char *s;
for (s = options; *s; s++)
{
@@ -1471,15 +1438,15 @@
}
-int main(argc, argv)
-int argc;
-char *argv[];
+int
+main(int argc, char *argv[])
{
struct stat sb;
FILE *log = stdout;
FILE *fp;
- int fd[3], doread, n, i;
- int tr, nr, regular[3], c;
+ int fd[3], doread;
+ size_t n, i, tr, nr;
+ int regular[3], c;
int fdt[3], devices = 0, make_daemon = 0;
char buf[DEFAULT_IPFLOGSIZE], *iplfile[3], *prog;
extern int optind;
Index: src/dist/ipf/tools/ipmon_y.y
===================================================================
RCS file: /cvsroot/src/dist/ipf/tools/Attic/ipmon_y.y,v
retrieving revision 1.2
diff -u -d -r1.2 ipmon_y.y
--- src/dist/ipf/tools/ipmon_y.y 15 Feb 2012 17:55:11 -0000 1.2
+++ src/dist/ipf/tools/ipmon_y.y 25 Oct 2012 02:08:38 -0000
@@ -454,7 +454,7 @@
int check_action(buf, log, opts, lvl)
-char *buf, *log;
+const char *buf, *log;
int opts, lvl;
{
ipmon_action_t *a;
----Next_Part(Thu_Oct_25_22_43_28_2012_891)----
From: christos@zoulas.com (Christos Zoulas)
To: Takeshi Nakayama <nakayama@NetBSD.org>
Cc: jnemeth@victoria.tc.ca, gnats-bugs@NetBSD.org, darrenr@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, martin@NetBSD.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Thu, 25 Oct 2012 11:35:18 -0400
On Oct 25, 10:43pm, nakayama@NetBSD.org (Takeshi Nakayama) wrote:
-- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
| I backported your fixes[*] to the netbsd-6 branch as attached. If
| there are no objections, I will send a pullup request.
|
| [*] http://mail-index.netbsd.org/source-changes/2012/10/21/msg038121.html
| http://mail-index.netbsd.org/source-changes/2012/10/22/msg038125.html
| http://mail-index.netbsd.org/source-changes/2012/10/22/msg038126.html
Go for it!
Thanks!
christos
From: Takeshi Nakayama <nakayama@NetBSD.org>
To: christos@zoulas.com
Cc: jnemeth@victoria.tc.ca, gnats-bugs@NetBSD.org, darrenr@NetBSD.org,
gnats-admin@NetBSD.org, netbsd-bugs@NetBSD.org, martin@NetBSD.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Fri, 26 Oct 2012 05:25:51 +0900 (JST)
>>> christos@zoulas.com (Christos Zoulas) wrote
> On Oct 25, 10:43pm, nakayama@NetBSD.org (Takeshi Nakayama) wrote:
> -- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
>
> | I backported your fixes[*] to the netbsd-6 branch as attached. If
> | there are no objections, I will send a pullup request.
> |
> | [*] http://mail-index.netbsd.org/source-changes/2012/10/21/msg038121.html
> | http://mail-index.netbsd.org/source-changes/2012/10/22/msg038125.html
> | http://mail-index.netbsd.org/source-changes/2012/10/22/msg038126.html
>
> Go for it!
Ah, but I found these changes don't fix the issue. Unaligned
buffers are still passed to print_ipflog(), print_natlog() and
print_statelog().
http://nxr.netbsd.org/xref/src/external/bsd/ipf/dist/tools/ipmon.c#1016
We need something as below.
Index: ipmon.c
===================================================================
RCS file: /cvsroot/src/external/bsd/ipf/dist/tools/ipmon.c,v
retrieving revision 1.5
diff -u -d -r1.5 ipmon.c
--- ipmon.c 22 Oct 2012 04:35:17 -0000 1.5
+++ ipmon.c 25 Oct 2012 20:23:59 -0000
@@ -130,9 +130,9 @@
static void handlehup(int);
static void flushlogs(const char *, FILE *);
static void print_log(config_t *, logsource_t *, const void *, size_t);
-static void print_ipflog(config_t *, const void *, size_t);
-static void print_natlog(config_t *, const void *, size_t);
-static void print_statelog(config_t *, const void *, size_t);
+static void print_ipflog(config_t *, const void *, const iplog_t *);
+static void print_natlog(config_t *, const void *, const iplog_t *);
+static void print_statelog(config_t *, const void *, const iplog_t *);
static int read_log(int, size_t *, void *, size_t);
static void write_pid(const char *);
static char *icmpname(u_int, u_int);
@@ -649,20 +649,18 @@
}
static void
-print_natlog(config_t *conf, const void *buf, size_t blen)
+print_natlog(config_t *conf, const void *buf, const iplog_t *ipl)
{
static u_32_t seqnum = 0;
int res, i, len, family;
const struct natlog *nl;
struct tm *tm;
- const iplog_t *ipl;
char *proto;
int simple;
char *t;
t = line;
simple = 0;
- ipl = (const iplog_t *)buf;
if (ipl->ipl_seqnum != seqnum) {
if ((ipmonopts & IPMON_SYSLOG) != 0) {
syslog(LOG_WARNING,
@@ -678,7 +676,7 @@
}
seqnum = ipl->ipl_seqnum + ipl->ipl_count;
- nl = (const struct natlog *)((const char *)ipl + sizeof(*ipl));
+ nl = (const struct natlog *)((const char *)buf + sizeof(*ipl));
res = (ipmonopts & IPMON_RESOLVE) ? 1 : 0;
tm = get_tm(ipl->ipl_sec);
len = sizeof(line);
@@ -837,17 +835,15 @@
static void
-print_statelog(config_t *conf, const void *buf, size_t blen)
+print_statelog(config_t *conf, const void *buf, const iplog_t *ipl)
{
static u_32_t seqnum = 0;
int res, i, len, family;
const struct ipslog *sl;
char *t, *proto;
struct tm *tm;
- const iplog_t *ipl;
t = line;
- ipl = (const iplog_t *)buf;
if (ipl->ipl_seqnum != seqnum) {
if ((ipmonopts & IPMON_SYSLOG) != 0) {
syslog(LOG_WARNING,
@@ -863,7 +859,7 @@
}
seqnum = ipl->ipl_seqnum + ipl->ipl_count;
- sl = (const struct ipslog *)((const char *)ipl + sizeof(*ipl));
+ sl = (const struct ipslog *)((const char *)buf + sizeof(*ipl));
res = (ipmonopts & IPMON_RESOLVE) ? 1 : 0;
tm = get_tm(ipl->ipl_sec);
len = sizeof(line);
@@ -1013,16 +1009,16 @@
switch (log->logtype) {
case IPL_LOGIPF:
if (ipl.ipl_magic == IPL_MAGIC)
- print_ipflog(conf, buf, psize);
+ print_ipflog(conf, buf, &ipl);
break;
case IPL_LOGNAT:
if (ipl.ipl_magic == IPL_MAGIC_NAT)
- print_natlog(conf, buf, psize);
+ print_natlog(conf, buf, &ipl);
break;
case IPL_LOGSTATE:
if (ipl.ipl_magic == IPL_MAGIC_STATE)
- print_statelog(conf, buf, psize);
+ print_statelog(conf, buf, &ipl);
break;
}
@@ -1033,7 +1029,7 @@
static void
-print_ipflog(config_t *conf, const void *buf, size_t blen)
+print_ipflog(config_t *conf, const void *buf, const iplog_t *ipl)
{
static u_32_t seqnum = 0;
int i, f, lvl, res, len, off, plen, ipoff, defaction;
@@ -1045,7 +1041,6 @@
u_32_t *s, *d;
u_short hl, p;
const ipflog_t *ipf;
- const iplog_t *ipl;
tcphdr_t *tp;
#ifdef USE_INET6
struct ip6_ext *ehp;
@@ -1054,7 +1049,6 @@
int go;
#endif
- ipl = (const iplog_t *)buf;
if (ipl->ipl_seqnum != seqnum) {
if ((ipmonopts & IPMON_SYSLOG) != 0) {
syslog(LOG_WARNING,
-- Takeshi Nakayama
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, darrenr@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, nakayama@netbsd.org
Cc:
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Thu, 25 Oct 2012 20:39:59 -0400
On Oct 25, 10:15pm, nakayama@NetBSD.org (Takeshi Nakayama) wrote:
-- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
| Ah, but I found these changes don't fix the issue. Unaligned
| buffers are still passed to print_ipflog(), print_natlog() and
| print_statelog().
|
| http://nxr.netbsd.org/xref/src/external/bsd/ipf/dist/tools/ipmon.c#1016
|
| We need something as below.
Ok, commit it and pull it up. Aren't there more structure casts?
christos
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: darrenr@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
nakayama@netbsd.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Fri, 26 Oct 2012 07:37:07 +0100
On Thu, Oct 25, 2012 at 10:15:05PM +0000, Takeshi Nakayama wrote:
> We need something as below.
Does that even compile ?
> +print_natlog(config_t *conf, const void *buf, const iplog_t *ipl)
> { ... }
> +print_statelog(config_t *conf, const void *buf, const iplog_t *ipl)
> { ...
> + print_ipflog(conf, buf, &ipl);
...
> + print_natlog(conf, buf, &ipl);
...
> + print_statelog(conf, buf, &ipl);
> ... }
>
> +print_ipflog(config_t *conf, const void *buf, const iplog_t *ipl)
{ ... }
I also can't see where you are doing the 'realignment copy'.
David
--
David Laight: david@l8s.co.uk
From: nakayama@NetBSD.org
To: david@l8s.co.uk
Cc: gnats-bugs@NetBSD.org, darrenr@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Fri, 26 Oct 2012 17:31:52 +0900 (JST)
> On Thu, Oct 25, 2012 at 10:15:05PM +0000, Takeshi Nakayama wrote:
>> We need something as below.
>
> Does that even compile ?
Yes.
>> +print_natlog(config_t *conf, const void *buf, const iplog_t *ipl)
>> { ... }
>
>> +print_statelog(config_t *conf, const void *buf, const iplog_t *ipl)
>> { ...
>> + print_ipflog(conf, buf, &ipl);
> ...
>> + print_natlog(conf, buf, &ipl);
> ...
>> + print_statelog(conf, buf, &ipl);
>> ... }
>>
>> +print_ipflog(config_t *conf, const void *buf, const iplog_t *ipl)
> { ... }
>
> I also can't see where you are doing the 'realignment copy'.
'iplog_t ipl' in print_log().
http://nxr.netbsd.org/xref/src/external/bsd/ipf/dist/tools/ipmon.c#1003
Then it is passed to print_ipflog(), print_natlog() and print_statelog().
-- Takeshi Nakayama
From: christos@zoulas.com (Christos Zoulas)
To: David Laight <david@l8s.co.uk>, gnats-bugs@NetBSD.org
Cc: darrenr@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
nakayama@netbsd.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Fri, 26 Oct 2012 08:34:48 -0400
On Oct 26, 7:37am, david@l8s.co.uk (David Laight) wrote:
-- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
| On Thu, Oct 25, 2012 at 10:15:05PM +0000, Takeshi Nakayama wrote:
| > We need something as below.
|
| Does that even compile ?
|
| > +print_natlog(config_t *conf, const void *buf, const iplog_t *ipl)
| > { ... }
|
| > +print_statelog(config_t *conf, const void *buf, const iplog_t *ipl)
| > { ...
| > + print_ipflog(conf, buf, &ipl);
| ...
| > + print_natlog(conf, buf, &ipl);
| ...
| > + print_statelog(conf, buf, &ipl);
| > ... }
| >
| > +print_ipflog(config_t *conf, const void *buf, const iplog_t *ipl)
| { ... }
|
| I also can't see where you are doing the 'realignment copy'.
I'll fix it.
christos
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, darrenr@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, nakayama@netbsd.org
Cc:
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Fri, 26 Oct 2012 08:37:16 -0400
On Oct 26, 8:35am, nakayama@NetBSD.org (nakayama@NetBSD.org) wrote:
-- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
|
| http://nxr.netbsd.org/xref/src/external/bsd/ipf/dist/tools/ipmon.c#1003
|
| Then it is passed to print_ipflog(), print_natlog() and print_statelog().
But there are other structure casts: ipflog_t, ip_t, etc.
christos
From: Takeshi Nakayama <nakayama@NetBSD.org>
To: christos@zoulas.com
Cc: gnats-bugs@NetBSD.org, darrenr@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
Date: Sat, 27 Oct 2012 06:11:46 +0900 (JST)
>>> christos@zoulas.com (Christos Zoulas) wrote
> On Oct 26, 8:35am, nakayama@NetBSD.org (nakayama@NetBSD.org) wrote:
> -- Subject: Re: bin/47101 (ipmon(8) alignment problem on 32-bit sparc)
>
> |
> | http://nxr.netbsd.org/xref/src/external/bsd/ipf/dist/tools/ipmon.c#1003
> |
> | Then it is passed to print_ipflog(), print_natlog() and print_statelog().
>
> But there are other structure casts: ipflog_t, ip_t, etc.
Ah yes, it just happens to work well on my machine. There are
other possibilities for alignemt issue.
-- Takeshi Nakayama
Responsible-Changed-From-To: darrenr->nakayama
Responsible-Changed-By: nakayama@NetBSD.org
Responsible-Changed-When: Sat, 27 Oct 2012 20:11:00 +0000
Responsible-Changed-Why:
christos fixed, and I'll manage a pullup.
State-Changed-From-To: open->pending-pullups
State-Changed-By: nakayama@NetBSD.org
State-Changed-When: Sun, 28 Oct 2012 00:13:05 +0000
State-Changed-Why:
pullup-6 #651
State-Changed-From-To: pending-pullups->closed
State-Changed-By: nakayama@NetBSD.org
State-Changed-When: Mon, 19 Nov 2012 19:11:17 +0000
State-Changed-Why:
http://mail-index.netbsd.org/source-changes/2012/11/19/msg038880.html
http://mail-index.netbsd.org/source-changes/2012/11/19/msg038881.html
pulled up.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.