From patchwork Sat Oct 6 06:45:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 1556791 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id C0877DFE80 for ; Sat, 6 Oct 2012 06:47:57 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TKO8u-00048c-JD; Sat, 06 Oct 2012 06:45:44 +0000 Received: from moutng.kundenserver.de ([212.227.126.171]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TKO8r-00048O-16 for linux-arm-kernel@lists.infradead.org; Sat, 06 Oct 2012 06:45:41 +0000 Received: from klappe2.localnet (HSI-KBW-149-172-5-253.hsi13.kabel-badenwuerttemberg.de [149.172.5.253]) by mrelayeu.kundenserver.de (node=mreu0) with ESMTP (Nemesis) id 0MWONS-1SvauH3SH4-00X5s9; Sat, 06 Oct 2012 08:45:08 +0200 From: Arnd Bergmann To: Julian Anastasov Subject: Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages Date: Sat, 6 Oct 2012 06:45:04 +0000 User-Agent: KMail/1.12.2 (Linux/3.5.0; KDE/4.3.2; x86_64; ; ) References: <1349448930-23976-1-git-send-email-arnd@arndb.de> <1349448930-23976-9-git-send-email-arnd@arndb.de> In-Reply-To: MIME-Version: 1.0 Message-Id: <201210060645.05225.arnd@arndb.de> X-Provags-ID: V02:K0:Vb6VrbmzCH10HtmGcnUOEWerJ6HNRDUNbKR7UIly73C c9r6YCj+v/stUxXael3ulR8SoQ456jDauo/Q/CpQZx9AglAsac PrQbt+9uQC7jfubAIAO6QnTc/psdYcOPBBIJvF1sz0gllKGoNX /BpxWiNc2Y+vlexKarKWW3gb34TSfWvbAverSfGNk0GSL8mppB AJDKtdVZTZDfFFbgiNERagz0zqM5+636fho+DmESFlLYxIGxEV e2dZxrANQXLpUydMsOhSDyRYbTCYen8pj9aZ6X7C4TrLv+1oPc z2Vvzzo2/C/00FtOPSr9iyhbifxTfI3RInOyPV7zq0JJ5nW9y7 Lcgv5chtnjzrtsit7a1g= X-Spam-Note: CRM114 invocation failed X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [212.227.126.171 listed in list.dnswl.org] -0.0 SPF_HELO_PASS SPF: HELO matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: netfilter@vger.kernel.org, coreteam@netfilter.org, arm@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Simon Horman , netfilter-devel@vger.kernel.org, "David S. Miller" , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On Friday 05 October 2012, Julian Anastasov wrote: > > Hello, > > On Fri, 5 Oct 2012, Arnd Bergmann wrote: > > > The ip_vs_set_timeout function sets timeouts for TCP and UDP, which > > can be enabled independently at compile time. The debug message > > always prints both timeouts that are passed into the function, > > but if one is disabled, the message will show uninitialized data. > > > > This splits the debug message into two separte IP_VS_DBG statements > > that are in the same #ifdef section to ensure we only print the > > text about what is actually going on. > > > > Without this patch, building ARM ixp4xx_defconfig results in: > > Are there any CONFIG_IP_VS_PROTO_xxx options in this > default config? It is a waste of memory if IPVS is compiled > without any protocols. They all appear to be turned off: $ grep CONFIG_IP_VS obj-tmp/.config CONFIG_IP_VS=m CONFIG_IP_VS_DEBUG=y CONFIG_IP_VS_TAB_BITS=12 # CONFIG_IP_VS_PROTO_TCP is not set # CONFIG_IP_VS_PROTO_UDP is not set # CONFIG_IP_VS_PROTO_AH_ESP is not set # CONFIG_IP_VS_PROTO_ESP is not set # CONFIG_IP_VS_PROTO_AH is not set # CONFIG_IP_VS_PROTO_SCTP is not set CONFIG_IP_VS_RR=m CONFIG_IP_VS_WRR=m CONFIG_IP_VS_LC=m CONFIG_IP_VS_WLC=m CONFIG_IP_VS_LBLC=m CONFIG_IP_VS_LBLCR=m CONFIG_IP_VS_DH=m CONFIG_IP_VS_SH=m # CONFIG_IP_VS_SED is not set # CONFIG_IP_VS_NQ is not set CONFIG_IP_VS_SH_TAB_BITS=8 > > net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd': > > net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] > > net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here > > net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] > > net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here > > net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] > > net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here > > There are many __ip_vs_get_timeouts callers but > just one calls memset(&t, 0, sizeof(t)) before that, > problem only for ip_vs_genl_set_config and ip_vs_set_timeout. > > To be safe, can we move this memset into > __ip_vs_get_timeouts instead of playing games with defines?: > > memset(t, 0, sizeof(*t)); > > This debug message will be more precise in showing the > changed values if we replace the __ip_vs_get_timeouts > call in ip_vs_genl_set_config with memset(&t, 0, sizeof(t)). > Then we will see 0 for values that are not changed/supported. 8<----- ipvs: initialize returned data in do_ip_vs_get_ctl As reported by a gcc warning, the do_ip_vs_get_ctl does not initalize all the members of the ip_vs_timeout_user structure it returns if at least one of the TCP or UDP protocols is disabled for ipvs. This makes sure that the data is always initialized, before it is returned as a response to IPVS_CMD_GET_CONFIG or printed as a debug message in IPVS_CMD_SET_CONFIG. Without this patch, building ARM ixp4xx_defconfig results in: net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd': net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here Signed-off-by: Arnd Bergmann Acked-by: Julian Anastasov diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index 2770f85..3995d2e 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -2590,6 +2588,7 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u) #if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP) struct ip_vs_proto_data *pd; #endif + memset(u, 0, sizeof (*u)); #ifdef CONFIG_IP_VS_PROTO_TCP pd = ip_vs_proto_data_get(net, IPPROTO_TCP); @@ -2768,7 +2767,6 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) { struct ip_vs_timeout_user t; - memset(&t, 0, sizeof(t)); __ip_vs_get_timeouts(net, &t); if (copy_to_user(user, &t, sizeof(t)) != 0) ret = -EFAULT;