Patchwork Fixed race condition at ip_vs.ko module init.

login
register
mail settings
Submitter Eduardo Blanco
Date Oct. 19, 2010, 9:26 a.m.
Message ID <AANLkTinVcSK0Dqx4Pkp1dZskR8YmkGYkkrSNgDqX6psW@mail.gmail.com>
Download mbox | patch
Permalink /patch/68291/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Eduardo Blanco - Oct. 19, 2010, 9:26 a.m.
Lists were initialized after the module was registered.  Multiple ipvsadm
processes at module load triggered a race condition that resulted in a null
pointer dereference in do_ip_vs_get_ctl(). As a result, __ip_vs_mutex
was left locked preventing all further ipvsadm commands.

Signed-off-by: Eduardo J. Blanco <ejblanco@google.com>
---
 net/netfilter/ipvs/ip_vs_ctl.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)
Simon Horman - Oct. 19, 2010, 3:23 p.m.
On Tue, Oct 19, 2010 at 10:26:47AM +0100, Eduardo Blanco wrote:
> Lists were initialized after the module was registered.  Multiple ipvsadm
> processes at module load triggered a race condition that resulted in a null
> pointer dereference in do_ip_vs_get_ctl(). As a result, __ip_vs_mutex
> was left locked preventing all further ipvsadm commands.
> 
> Signed-off-by: Eduardo J. Blanco <ejblanco@google.com>

Thanks Eduardo.

Patrick, please consider pulling

git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick

to get this change.

> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 0f0c079..68624dc 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3385,6 +3385,16 @@ int __init ip_vs_control_init(void)
> 
>  	EnterFunction(2);
> 
> +	/* Initialize ip_vs_svc_table, ip_vs_svc_fwm_table, ip_vs_rtable */
> +	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
> +		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> +		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> +	}
> +	for(idx = 0; idx < IP_VS_RTAB_SIZE; idx++)  {
> +		INIT_LIST_HEAD(&ip_vs_rtable[idx]);
> +	}
> +	smp_wmb();
> +
>  	ret = nf_register_sockopt(&ip_vs_sockopts);
>  	if (ret) {
>  		pr_err("cannot register sockopt.\n");
> @@ -3403,15 +3413,6 @@ int __init ip_vs_control_init(void)
> 
>  	sysctl_header = register_sysctl_paths(net_vs_ctl_path, vs_vars);
> 
> -	/* Initialize ip_vs_svc_table, ip_vs_svc_fwm_table, ip_vs_rtable */
> -	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
> -		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> -		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> -	}
> -	for(idx = 0; idx < IP_VS_RTAB_SIZE; idx++)  {
> -		INIT_LIST_HEAD(&ip_vs_rtable[idx]);
> -	}
> -
>  	ip_vs_new_estimator(&ip_vs_stats);
> 
>  	/* Hook the defense timer */
> -- 
> 1.7.1
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy - Oct. 21, 2010, 8:31 a.m.
Am 19.10.2010 17:23, schrieb Simon Horman:
> On Tue, Oct 19, 2010 at 10:26:47AM +0100, Eduardo Blanco wrote:
>> Lists were initialized after the module was registered.  Multiple ipvsadm
>> processes at module load triggered a race condition that resulted in a null
>> pointer dereference in do_ip_vs_get_ctl(). As a result, __ip_vs_mutex
>> was left locked preventing all further ipvsadm commands.
>>
>> Signed-off-by: Eduardo J. Blanco <ejblanco@google.com>
> 
> Thanks Eduardo.
> 
> Patrick, please consider pulling
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick
> 
> to get this change.

Pulled, thanks.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 0f0c079..68624dc 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3385,6 +3385,16 @@  int __init ip_vs_control_init(void)

 	EnterFunction(2);

+	/* Initialize ip_vs_svc_table, ip_vs_svc_fwm_table, ip_vs_rtable */
+	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
+		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
+		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
+	}
+	for(idx = 0; idx < IP_VS_RTAB_SIZE; idx++)  {
+		INIT_LIST_HEAD(&ip_vs_rtable[idx]);
+	}
+	smp_wmb();
+
 	ret = nf_register_sockopt(&ip_vs_sockopts);
 	if (ret) {
 		pr_err("cannot register sockopt.\n");
@@ -3403,15 +3413,6 @@  int __init ip_vs_control_init(void)

 	sysctl_header = register_sysctl_paths(net_vs_ctl_path, vs_vars);

-	/* Initialize ip_vs_svc_table, ip_vs_svc_fwm_table, ip_vs_rtable */
-	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
-		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
-		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
-	}
-	for(idx = 0; idx < IP_VS_RTAB_SIZE; idx++)  {
-		INIT_LIST_HEAD(&ip_vs_rtable[idx]);
-	}
-
 	ip_vs_new_estimator(&ip_vs_stats);

 	/* Hook the defense timer */