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:

NetBSD Home
NetBSD PR Database Search

(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.