From patchwork Sat Oct 6 09:54:15 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 1557101 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 3D2F73FD56 for ; Sat, 6 Oct 2012 09:57:42 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1TKR5e-0007ZX-DE; Sat, 06 Oct 2012 09:54:34 +0000 Received: from moutng.kundenserver.de ([212.227.17.9]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1TKR5Z-0007ZJ-1X for linux-arm-kernel@lists.infradead.org; Sat, 06 Oct 2012 09:54:29 +0000 Received: from klappe2.localnet (HSI-KBW-149-172-5-253.hsi13.kabel-badenwuerttemberg.de [149.172.5.253]) by mrelayeu.kundenserver.de (node=mreu3) with ESMTP (Nemesis) id 0MLEbl-1TK9yF26Wv-000Ik8; Sat, 06 Oct 2012 11:54:18 +0200 From: Arnd Bergmann To: Julian Anastasov , Krzysztof Halasa Subject: Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages Date: Sat, 6 Oct 2012 09:54:15 +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> <201210060645.05225.arnd@arndb.de> In-Reply-To: MIME-Version: 1.0 Message-Id: <201210060954.15831.arnd@arndb.de> X-Provags-ID: V02:K0:RnJ2vlE1oskKEax+YlApCrZBO76YyLrbB3SXjUqma2D +Bw8TsWyNiEgB54l9iKZhSyUl0Fosk2wItsAkvlHHU9wuMZEnX wsZdqivxFjXhwZUNZ0DkS8k0QiP0v5V/7a7QdzAH3fzWehG99J 2QDUsdskkYQNdLcALZfss9/mB6yHUqgzwITsTPuqs3hRqj7nc9 7oNre+IKBxC785UX0VKOVZUwwy15dvDGP30dl9Ax+KVU6NOH/p 4U3rQTn3e5R3edbA5vfzxIdSXd8kcg2n4uS5DzveZgC4+I1/x3 Ecy4Ffyb1PvLV8iE970983C9SIl0YMWllDAW4B9+Y6W+ARMSR6 w8uc4+Hr8mXJ9/Kx+e3Q= 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.17.9 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 Saturday 06 October 2012, Julian Anastasov wrote: > On Sat, 6 Oct 2012, Arnd Bergmann wrote: > > > 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 > > Something should be changed here, may be at least > TCP/UDP, who knows. I don't try to read too much into our defconfigs. We have 140 of them on ARM, and they are mainly useful to give a reasonable build coverage, but I wouldn't expect them to be actually used on that hardware. I'll leave it up to Krzysztof to send a patch for this if he wants. > > --- 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 > > That is what we want. If you plan another submission > you can add empty line before this memset and to replace > the __ip_vs_get_timeouts call in ip_vs_genl_set_config with > memset but they are cosmetic changes. Or may be Simon will > take care about the coding style when applying the change. > > Acked-by: Julian Anastasov I'd prefer Simon to pick up the patch. He should also decide whether he wants to add it to stable. In theory, this is a small leak of kernel stack data to user space, but as you say in practice it should not happen because it only exists for silly configurations that nobody should be using. AFAICT, removing the call to __ip_vs_get_timeouts in do_ip_vs_get_ctl would be a semantic change for the case where a user sends a IPVS_CMD_SET_CONFIG message without without the complete set of attributes inside it. The current behavior is to leave the timeouts alone, replacing the __ip_vs_get_timeouts with a memset would zero them. I left this part alone then. Arnd 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..c4ee437 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -2591,6 +2589,8 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u) 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); u->tcp_timeout = pd->timeout_table[IP_VS_TCP_S_ESTABLISHED] / HZ; @@ -2768,7 +2768,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;