Date: Mon, 7 Dec 2009 10:37:13 -0800 From: Pyun YongHyeon <pyunyh@gmail.com> To: Mark Abene <phiber@phiber.com> Cc: freebsd-net@freebsd.org, darren@FreeBSD.org, bug-followup@FreeBSD.org Subject: Re: kern/106438: [ipf] ipfilter: keep state does not seem to allow replies in on spar64 (and maybe others) Message-ID: <20091207183712.GA1366@michelle.cdnetworks.com> In-Reply-To: <200912070140.nB71e4Kw039201@freefall.freebsd.org> References: <200912070140.nB71e4Kw039201@freefall.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--C7zPtVaVf+AK4Oqc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Mon, Dec 07, 2009 at 01:40:04AM +0000, Mark Abene wrote:
> The following reply was made to PR kern/106438; it has been noted by GNATS.
>
> From: Mark Abene <phiber@phiber.com>
> To: bug-followup@FreeBSD.org, mala@hinterbergen.de
> Cc:
> Subject: Re: kern/106438: [ipf] ipfilter: keep state does not seem to allow
> replies in on spar64 (and maybe others)
> Date: Sun, 06 Dec 2009 20:26:25 -0500
>
> It's been several years since this was first reported, and I can confirm
> that it's still a problem in FreeBSD 8.0-RELEASE on i386 with an fxp
> interface. I just wasted nearly two days trying to figure out why our
> ipfilter rules which have been in use for years on our firewall suddenly
> locked the machine out when we upgraded from a rather old version of
> FreeBSD to 8.0-RELEASE.
>
> Same exact problem, same exact symptoms. Disabling checksumming on the
> interface resolved the problem completely, otherwise ipfilter was rather
> broken. I'm really glad I found this bug report, though not soon
> enough. This is a rather serious problem.
>
I think the bug is in ipfilter's checksum computation. Unlike other
operating systems, FreeBSD also supports cheap controllers that lacks
pseudo checksum operation. These controllers just compute partial
checksum without pseudo header and drivers(fxp(4), hme(4) and gem(4))
that take advantage of this feature insert a tag which indicates
pseudo checksum is required in upper stack. The checksum computation
code in ipfilter didn't account for IP header length so it always
computed checksum wrong.
I guess the following patch may fix the issue. The patch is not
tested and wouldn't be complete as it assumes IPv4. However no other
driver in tree set CSUM_DATA_VALID without CSUM_PSEUDO_HDR for IPv6
at this moment.
Darren, would you review the patch? Because ipfilter lives in
contrib I think it should go upstream first.
> -Mark
--C7zPtVaVf+AK4Oqc
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="ipfilter.csum.patch"
Index: sys/contrib/ipfilter/netinet/ip_fil_freebsd.c
===================================================================
--- sys/contrib/ipfilter/netinet/ip_fil_freebsd.c (revision 200222)
+++ sys/contrib/ipfilter/netinet/ip_fil_freebsd.c (working copy)
@@ -1357,7 +1357,9 @@
else
sum = in_pseudo(ip->ip_src.s_addr, ip->ip_dst.s_addr,
htonl(m->m_pkthdr.csum_data +
- fin->fin_ip->ip_len + fin->fin_p));
+ fin->fin_ip->ip_len -
+ (fin->fin_ip->ip_hl << 2) +
+ fin->fin_p));
sum ^= 0xffff;
if (sum != 0) {
fin->fin_flx |= FI_BAD;
--C7zPtVaVf+AK4Oqc--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091207183712.GA1366>