From patchwork Wed Oct 3 20:33:38 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: 188915 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id CA7602C045A for ; Thu, 4 Oct 2012 06:33:47 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753595Ab2JCUdq (ORCPT ); Wed, 3 Oct 2012 16:33:46 -0400 Received: from mail.us.es ([193.147.175.20]:38978 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753485Ab2JCUdp (ORCPT ); Wed, 3 Oct 2012 16:33:45 -0400 Received: (qmail 5771 invoked from network); 3 Oct 2012 22:33:42 +0200 Received: from unknown (HELO us.es) (192.168.2.12) by us.es with SMTP; 3 Oct 2012 22:33:42 +0200 Received: (qmail 19682 invoked by uid 507); 3 Oct 2012 20:33:42 -0000 X-Qmail-Scanner-Diagnostics: from 127.0.0.1 by antivirus2 (envelope-from , uid 501) with qmail-scanner-2.10 (clamdscan: 0.97.6/15427. spamassassin: 3.3.2. Clear:RC:1(127.0.0.1):SA:0(-99.7/7.5):. Processed in 3.220099 secs); 03 Oct 2012 20:33:42 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on antivirus2 X-Spam-Level: X-Spam-Status: No, score=-99.7 required=7.5 tests=BAYES_50, RP_MATCHES_RCVD, SPF_HELO_FAIL, T_FRT_CONTACT, USER_IN_WHITELIST autolearn=disabled version=3.3.2 X-Envelope-From: pneira@us.es Received: from unknown (HELO antivirus2) (127.0.0.1) by us.es with SMTP; 3 Oct 2012 20:33:39 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus2 (F-Secure/fsigk_smtp/407/antivirus2); Wed, 03 Oct 2012 22:33:39 +0200 (CEST) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus2) Received: (qmail 23562 invoked from network); 3 Oct 2012 22:33:38 +0200 Received: from 1984.lsi.us.es (HELO us.es) (1984lsi@150.214.188.80) by us.es with AES128-SHA encrypted SMTP; 3 Oct 2012 22:33:38 +0200 Date: Wed, 3 Oct 2012 22:33:38 +0200 From: Pablo Neira Ayuso To: "Gutholm, James" Cc: "netfilter-devel@vger.kernel.org" Subject: Re: Conntrackd Segmentation fault due to nfexp_get_attr returning NULL Message-ID: <20121003203338.GA10154@1984> References: <3E070A3A9446DB43B8A316366DBEABBE1F26A591@palm.evergreen.edu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <3E070A3A9446DB43B8A316366DBEABBE1F26A591@palm.evergreen.edu> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org On Wed, Oct 03, 2012 at 06:52:08PM +0000, Gutholm, James wrote: > > Under heavy load conntrackd is crashing. Running under gdb I was able to determine that the crashes are caused by an unchecked null pointer returned by nfexp_get_attr in both exp_filter_find() in filter.c and exp_build_str() in build.c > i > This only happens when expectation sync is being used. Setting "ExpectationSync Off" in conntrackd.conf stops the crashes. > > I coded in a couple of checks on the pointer returned which at least stop the errors. I've included the changes as diffs and also the gdb output in case it is helpful. If there's something else I can provide, I'm happy to help but this might be pushing the limit of my expertise. > > James > > This is on RHEL6 (2.6.32-300.32.2.el6uek.x86_64) with conntrack-tool built from source. I see, I forgot to document that Linux kernel >= 3.5 to get ExpectationSync working flawlessly is required. I have attached the following patch. It fixes the crash, and document this accordingly but you still will have to upgrade your kernel if you want expectation synchronization. BTW, thanks a lot for the report, really accurate. I'd appreciate if you give it a test, just to make sure we don't crash anymore, even if you will not get the expectsync feature working correctly in all possible scenarios. From 597d10a74acbb17bba5db48ca71c3a76ce2d9305 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Wed, 3 Oct 2012 22:19:25 +0200 Subject: [PATCH] conntrackd: fix crash if ExpectationSync is enabled on old Linux kernels ExpectationSync requires Linux kernel >= 3.5 to work sanely, document this. Still, we don't want to crash if someone enables expectation sync with old Linux kernels (like 2.6.32). Reported-by: James Gutholm Signed-off-by: Pablo Neira Ayuso --- doc/manual/conntrack-tools.tmpl | 7 +++++++ doc/sync/alarm/conntrackd.conf | 3 ++- doc/sync/ftfw/conntrackd.conf | 3 ++- doc/sync/notrack/conntrackd.conf | 3 ++- src/build.c | 3 ++- src/filter.c | 12 +++++++++++- 6 files changed, 26 insertions(+), 5 deletions(-) diff --git a/doc/manual/conntrack-tools.tmpl b/doc/manual/conntrack-tools.tmpl index 47e6f84..63a53e4 100644 --- a/doc/manual/conntrack-tools.tmpl +++ b/doc/manual/conntrack-tools.tmpl @@ -660,6 +660,13 @@ Sync { Synchronization of expectations + Check your Linux kernel version first + + The synchronization of expectations require a Linux kernel >= 3.5 + to work appropriately. + + + The connection tracking system provides helpers that allows you to filter multi-flow application protocols like FTP, H.323 and SIP among many others. These protocols usually split the control and data traffic in diff --git a/doc/sync/alarm/conntrackd.conf b/doc/sync/alarm/conntrackd.conf index b9520fb..0223745 100644 --- a/doc/sync/alarm/conntrackd.conf +++ b/doc/sync/alarm/conntrackd.conf @@ -194,7 +194,8 @@ Sync { # Set this option on if you want to enable the synchronization # of expectations. You have to specify the list of helpers that - # you want to enable. Default is off. + # you want to enable. Default is off. This feature requires + # a Linux kernel >= 3.5. # # ExpectationSync { # ftp diff --git a/doc/sync/ftfw/conntrackd.conf b/doc/sync/ftfw/conntrackd.conf index 53a7d0f..65e7b77 100644 --- a/doc/sync/ftfw/conntrackd.conf +++ b/doc/sync/ftfw/conntrackd.conf @@ -217,7 +217,8 @@ Sync { # Set this option on if you want to enable the synchronization # of expectations. You have to specify the list of helpers that - # you want to enable. Default is off. + # you want to enable. Default is off. This feature requires + # a Linux kernel >= 3.5. # # ExpectationSync { # ftp diff --git a/doc/sync/notrack/conntrackd.conf b/doc/sync/notrack/conntrackd.conf index 11f022e..3d036fb 100644 --- a/doc/sync/notrack/conntrackd.conf +++ b/doc/sync/notrack/conntrackd.conf @@ -256,7 +256,8 @@ Sync { # Set this option on if you want to enable the synchronization # of expectations. You have to specify the list of helpers that - # you want to enable. Default is off. + # you want to enable. Default is off. This feature requires + # a Linux kernel >= 3.5. # # ExpectationSync { # ftp diff --git a/src/build.c b/src/build.c index 7d4ef12..e15eb4f 100644 --- a/src/build.c +++ b/src/build.c @@ -356,7 +356,8 @@ void exp2msg(const struct nf_expect *exp, struct nethdr *n) exp_build_u32(exp, ATTR_EXP_NAT_DIR, n, NTA_EXP_NAT_DIR); } - exp_build_str(exp, ATTR_EXP_HELPER_NAME, n, NTA_EXP_HELPER_NAME); + if (nfexp_attr_is_set(exp, ATTR_EXP_HELPER_NAME)) + exp_build_str(exp, ATTR_EXP_HELPER_NAME, n, NTA_EXP_HELPER_NAME); if (nfexp_attr_is_set(exp, ATTR_EXP_FN)) exp_build_str(exp, ATTR_EXP_FN, n, NTA_EXP_FN); } diff --git a/src/filter.c b/src/filter.c index 39dd4ca..02a8078 100644 --- a/src/filter.c +++ b/src/filter.c @@ -473,7 +473,17 @@ int exp_filter_find(struct exp_filter *f, const struct nf_expect *exp) return 1; list_for_each_entry(item, &f->list, head) { - const char *name = nfexp_get_attr(exp, ATTR_EXP_HELPER_NAME); + const char *name; + + if (nfexp_attr_is_set(exp, ATTR_EXP_HELPER_NAME)) + name = nfexp_get_attr(exp, ATTR_EXP_HELPER_NAME); + else { + /* No helper name, this is likely to be a kernel older + * which does not include the helper name, just skip + * this so we don't crash. + */ + return 0; + } /* we allow partial matching to support things like sip-PORT. */ if (strncasecmp(item->helper_name, name, -- 1.7.10.4