Date: Sat, 26 Aug 2006 09:00:43 GMT From: "Stephen E. Halpin" <seh-10lzx4@mail.quadrizen.com> To: freebsd-ipfw@FreeBSD.org Subject: Re: bin/102422: ipfw & kernel problems where firewall rules aren't interpreted correctly Message-ID: <200608260900.k7Q90h9j006357@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/102422; it has been noted by GNATS.
From: "Stephen E. Halpin" <seh-10lzx4@mail.quadrizen.com>
To: Andrey V. Elsukov <bu7cher@yandex.ru>
Cc: bug-followup@FreeBSD.org,
Oleg Bulyzhin <oleg@FreeBSD.org>,
Gleb Smirnoff <glebius@FreeBSD.org>,
Luigi Rizzo <rizzo@icir.org>
Subject: Re: bin/102422: ipfw & kernel problems where firewall rules aren't interpreted correctly
Date: Sat, 26 Aug 2006 05:01:39 -0400
Sorry for taking so long to get back to you. The changes look good.
I've tested the changes for ip_fw2.c for both source and destination
processing, and it worked fine. I still have a question about PR
91245, as when I went to the following page:
http://www.freebsd.org/cgi/cvsweb.cgi/src/sbin/ipfw/
and it looks like the last version of ipfw2.c is 1.96 on the MAIN
branch, and the changes in PR 91245 are not there. It would be
awesome if all three fixes could make it into the 6.2 release!
-Steve
On Aug 24, 2006, at 6:09 AM, Andrey V. Elsukov wrote:
> Stephen Halpin wrote:
> > The rule is accepted with icmp6types 1,2,32,33,34,...94,95,128,129.
> > The problem is the data structure in
> > /usr/src/sbin/ipfw/ipfw2.c:fill_icmp6types() is not properly
> > initialized.
>
> Yes, you are right. A data buffer is previously zeroed, but
> fill_ip6() function can modified some data while parsing ipv6
> destination addresses. Quick fix is simple:
>
> --- ipfw2.c 23 Aug 2006 14:29:18 -0000 1.96
> +++ ipfw2.c 24 Aug 2006 09:08:06 -0000
> @@ -1206,7 +1206,7 @@
> {
> uint8_t type;
>
> - cmd->d[0] = 0;
> + bzero(cmd, sizeof(*cmd));
> while (*av) {
> if (*av == ',')
> av++;
>
>
> But i think that here can be another similar issues.
>
> > addressed with bug number 91245, which the query interface won't
> > bring up for anything. I was able to find it using the global
> > Google. I found a set of diffs at:
>
> PR 91245 was closed.
> http://www.freebsd.org/cgi/query-pr.cgi?pr=91245
>
> > Problem 3:
> >
> > ipfw add allow ip6 from any to 2000::/16,2002::/16
>
> Can you try the attached patch?
>
> --
> WBR, Andrey V. Elsukov
> Index: ip_fw2.c
> ===================================================================
> RCS file: /mnt/cvs/ncvs/src/sys/netinet/ip_fw2.c,v
> retrieving revision 1.144
> diff -u -r1.144 ip_fw2.c
> --- ip_fw2.c 18 Aug 2006 22:36:04 -0000 1.144
> +++ ip_fw2.c 24 Aug 2006 09:55:38 -0000
> @@ -2869,22 +2869,20 @@
> &((ipfw_insn_ip6 *)cmd)->addr6);
> break;
> case O_IP6_SRC_MASK:
> - if (is_ipv6) {
> - ipfw_insn_ip6 *te = (ipfw_insn_ip6 *)cmd;
> - struct in6_addr p = args->f_id.src_ip6;
> -
> - APPLY_MASK(&p, &te->mask6);
> - match = IN6_ARE_ADDR_EQUAL(&te->addr6, &p);
> - }
> - break;
> -
> case O_IP6_DST_MASK:
> if (is_ipv6) {
> - ipfw_insn_ip6 *te = (ipfw_insn_ip6 *)cmd;
> - struct in6_addr p = args->f_id.dst_ip6;
> + int i = cmdlen - 1;
> + struct in6_addr p;
> + struct in6_addr *d = &((ipfw_insn_ip6 *)cmd)->addr6;
>
> - APPLY_MASK(&p, &te->mask6);
> - match = IN6_ARE_ADDR_EQUAL(&te->addr6, &p);
> + for (; !match && i > 0; d += 2,
> + i -= F_INSN_SIZE(struct in6_addr) * 2)
> + {
> + p = (cmd->opcode == O_IP6_SRC_MASK) ?
> + args->f_id.src_ip6: args->f_id.dst_ip6;
> + APPLY_MASK(&p, &d[1]);
> + match = IN6_ARE_ADDR_EQUAL(&d[0], &p);
> + }
> }
> break;
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200608260900.k7Q90h9j006357>