From patchwork Sun Mar 4 11:32:50 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pablo Neira Ayuso X-Patchwork-Id: 144506 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 66635B6F9D for ; Sun, 4 Mar 2012 22:32:56 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753077Ab2CDLcz (ORCPT ); Sun, 4 Mar 2012 06:32:55 -0500 Received: from mail.us.es ([193.147.175.20]:49446 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753059Ab2CDLcx (ORCPT ); Sun, 4 Mar 2012 06:32:53 -0500 Received: (qmail 965 invoked from network); 4 Mar 2012 12:32:52 +0100 Received: from unknown (HELO us.es) (192.168.2.13) by us.es with SMTP; 4 Mar 2012 12:32:52 +0100 Received: (qmail 1360 invoked by uid 507); 4 Mar 2012 11:32:50 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on antivirus3 X-Spam-Level: X-Spam-Status: No, score=-99.2 required=7.5 tests=BAYES_50,SPF_HELO_FAIL, USER_IN_WHITELIST autolearn=disabled version=3.3.1 Received: from 127.0.0.1 by antivirus3 (envelope-from , uid 501) with qmail-scanner-2.08 (clamdscan: 0.97.3/14580. Clear:RC:1(127.0.0.1):. Processed in 0.054438 secs); 04 Mar 2012 11:32:50 -0000 Received: from unknown (HELO antivirus3) (127.0.0.1) by us.es with SMTP; 4 Mar 2012 11:32:50 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus3 (F-Secure/fsigk_smtp/407/antivirus3); Sun, 04 Mar 2012 12:32:50 +0100 (CET) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus3) Received: (qmail 755 invoked from network); 4 Mar 2012 12:32:50 +0100 Received: from 1984.lsi.us.es (HELO us.es) (1984lsi@150.214.188.80) by us.es with AES128-SHA encrypted SMTP; 4 Mar 2012 12:32:50 +0100 Date: Sun, 4 Mar 2012 12:32:50 +0100 From: Pablo Neira Ayuso To: Richard Weinberger Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, eric.dumazet@gmail.com, jengelh@medozas.de, rostedt@goodmis.org Subject: Re: [PATCH 1/4] Netfilter: Merge ipt_LOG and ip6_LOG into xt_LOG Message-ID: <20120304113250.GA22781@1984> References: <1328400892-22409-1-git-send-email-richard@nod.at> <1328400892-22409-3-git-send-email-richard@nod.at> <20120301112732.GA6806@1984> <4F4FEC83.6090504@nod.at> <20120302164945.GA13723@1984> <4F50FAE2.4080903@nod.at> <20120304111211.GA22592@1984> <4F535161.1010407@nod.at> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4F535161.1010407@nod.at> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sun, Mar 04, 2012 at 12:26:25PM +0100, Richard Weinberger wrote: > Am 04.03.2012 12:12, schrieb Pablo Neira Ayuso: > > On Fri, Mar 02, 2012 at 05:52:50PM +0100, Richard Weinberger wrote: > >> Am 02.03.2012 17:49, schrieb Pablo Neira Ayuso: > >>> On Thu, Mar 01, 2012 at 10:39:15PM +0100, Richard Weinberger wrote: > >>>> Am 01.03.2012 12:27, schrieb Pablo Neira Ayuso: > >>>>> While merging ipt_LOG and ip6t_LOG, you introduced some bug that > >>>>> corrupts the log line. Note the extra PROTO=, I don't have any UDPLITE > >>>>> traffic here. > >>>>> > >>>>> Looks like a missing break in one switch. > >>>> > >>>> I got confused by my own logic. :-\ > >>>> Does the attached patch fix the issue? > >>>> It's based on "Netfilter: xt_LOG: Add timestamp support" > >>> > >>> This patch lacks of description. If you don't make it myself, I have > >>> to do it for you :-( > >>> > >>> Please, send me patches following the standard format next time. > >> > >> It was a "does this patch solve the problem"-Patch. > >> Does it fix the problem? > >> > >> If so, I'll send an official one... > > > > Sorry, that's too much overhead. I don't mind testing it, but I want > > to apply it as soon as it fixes the problem ;-) > > > > I'll try to reproduce your problem and test the fix for my own. Here it works fine, but double test it fine, thanks. Here's the patch, I added the description. From 0bfff14a7d9b81dc2ddf5d7ea08d3fb11d0f67a9 Mon Sep 17 00:00:00 2001 From: Richard Weinberger Date: Thu, 1 Mar 2012 11:39:15 +0000 Subject: [PATCH] netfilter: xt_LOG: fix bogus extra layer-4 logging information In 16059b5 netfilter: merge ipt_LOG and ip6_LOG into xt_LOG, we have merged ipt_LOG and ip6t_LOG. However: IN=wlan0 OUT= MAC=xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx:xx SRC=213.150.61.61 DST=192.168.1.133 LEN=40 TOS=0x00 PREC=0x00 TTL=117 ID=10539 DF PROTO=TCP SPT=80 DPT=49013 WINDOW=0 RES=0x00 ACK RST URGP=0 PROTO=UDPLITE SPT=80 DPT=49013 LEN=45843 PROTO=ICMP TYPE=0 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Several missing break in the code led to including bogus layer-4 information. This patch fixes this problem. Signed-off-by: Richard Weinberger Signed-off-by: Pablo Neira Ayuso --- net/netfilter/xt_LOG.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/net/netfilter/xt_LOG.c b/net/netfilter/xt_LOG.c index 1595608..f99f8de 100644 --- a/net/netfilter/xt_LOG.c +++ b/net/netfilter/xt_LOG.c @@ -216,12 +216,14 @@ static void dump_ipv4_packet(struct sbuff *m, ntohs(ih->frag_off) & IP_OFFSET, iphoff+ih->ihl*4, logflags)) return; + break; case IPPROTO_UDP: case IPPROTO_UDPLITE: if (dump_udp_header(m, skb, ih->protocol, ntohs(ih->frag_off) & IP_OFFSET, iphoff+ih->ihl*4)) return; + break; case IPPROTO_ICMP: { struct icmphdr _icmph; const struct icmphdr *ich; @@ -649,10 +651,12 @@ static void dump_ipv6_packet(struct sbuff *m, if (dump_tcp_header(m, skb, currenthdr, fragment, ptr, logflags)) return; + break; case IPPROTO_UDP: case IPPROTO_UDPLITE: if (dump_udp_header(m, skb, currenthdr, fragment, ptr)) return; + break; case IPPROTO_ICMPV6: { struct icmp6hdr _icmp6h; const struct icmp6hdr *ic; -- 1.7.7.3