On Sat, Oct 06, 2012 at 09:54:15AM +0000, Arnd Bergmann wrote:
> 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 <ja@xxxxxx>
>
> 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
Hi,
sorry for being a bit slow, it was a long weekend here.
This patch looks reasonable and I think it is appropriate for stable.
I'll see about getting it merged accordingly.
>
> 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 <arnd@xxxxxxxx>
> Acked-by: Julian Anastasov <ja@xxxxxx>
> ---
> 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;
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
