From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752276AbdAZFk4 (ORCPT ); Thu, 26 Jan 2017 00:40:56 -0500 Received: from smtprelay0108.hostedemail.com ([216.40.44.108]:39742 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751203AbdAZFky (ORCPT ); Thu, 26 Jan 2017 00:40:54 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 50,3,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::::,RULES_HIT:41:355:379:541:599:968:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1542:1593:1594:1711:1730:1747:1777:1792:2393:2559:2562:2693:2828:2894:3138:3139:3140:3141:3142:3353:3622:3865:3867:3868:3871:3872:3874:4250:4321:5007:6119:7875:7903:10012:10400:10848:11026:11232:11473:11658:11914:12043:12296:12438:12740:12760:12895:13095:13161:13229:13255:13439:14093:14097:14659:14721:21080:21324:21433:21451:30012:30054:30060:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:2:0,LFtime:4,LUA_SUMMARY:none X-HE-Tag: mine07_24810efc7fd13 X-Filterd-Recvd-Size: 3923 Message-ID: <1485409249.12563.96.camel@perches.com> Subject: Re: [PATCH v2] netfilter: nf_ct_helper: warn when not applying default helper assignment From: Joe Perches To: Jiri Kosina , Linus Torvalds Cc: Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , NetFilter , coreteam@netfilter.org, Linux Kernel Mailing List , info@jablonka.cz, eric@regit.org Date: Wed, 25 Jan 2017 21:40:49 -0800 In-Reply-To: References: <20170124012859.GA6375@salvia> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.22.3-0ubuntu0.1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-01-25 at 21:43 +0100, Jiri Kosina wrote: > Rewrite the code a little bit as suggested by Linus, so that we avoid > spaghettiing the code even more -- namely the whole decision making > process regarding helper selection (either automatic or not) is being > separated, so that the whole logic can be simplified and code (condition) > duplication reduced. [] > diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c [] > @@ -188,6 +188,39 @@ struct nf_conn_help * > } > EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add); > > +static struct nf_conntrack_helper *find_auto_helper(struct nf_conn *ct) > +{ > + return __nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple); > +} > + > +static struct nf_conntrack_helper *ct_lookup_helper(struct nf_conn *ct, struct net *net) > +{ > + struct nf_conntrack_helper *ret; > + > + if (!net->ct.sysctl_auto_assign_helper) { > + if (net->ct.auto_assign_helper_warned) > + return NULL; > + if (!find_auto_helper(ct)) > + return NULL; > + pr_info("nf_conntrack: default automatic helper assignment " > + "has been turned off for security reasons and CT-based " > + " firewall rule not found. Use the iptables CT target " > + "to attach helpers instead.\n"); > + net->ct.auto_assign_helper_warned = 1; > + return NULL; > + } > + > + ret = find_auto_helper(ct); > + if (!ret || net->ct.auto_assign_helper_warned) > + return ret; > + pr_info("nf_conntrack: automatic helper assignment is deprecated and it will " > + "be removed soon. Use the iptables CT target to attach helpers " > + " instead.\n"); > + net->ct.auto_assign_helper_warned = 1; > + return ret; > +} There are whitespece defects concatenating these multi-line strings. How about an exit block that emits the message like { [...] const char *msg; [...] if (!net->ct.sysctl_auto_assign_helper) { if (net->ct.auto_assign_helper_warned) return NULL; if (!find_auto_helper(ct)) return NULL; msg = "default automatic helper assignment has been turned off for security reasons and CT-based firewall rule not found"; ret = NULL; } else { ret = find_auto_helper(ct); if (!ret || net->ct.auto_assign_helper_warned) return ret; msg = "automatic helper assignment is deprecated and it will be removed soon"; net->ct.auto_assign_helper_warned = 1; } pr_info("nf_conntrack: %s. Use the iptables CT target to attach helpers instead.\n", msg); return ret; }