linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* improper initialization of ipsec_table  in openswan 2.4.9
@ 2007-11-06 12:04 Eran Ben-Avi
  2007-11-08  0:29 ` David McCullough
  0 siblings, 1 reply; 3+ messages in thread
From: Eran Ben-Avi @ 2007-11-06 12:04 UTC (permalink / raw)
  To: linux-crypto

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

Hi,

While working with openswan 2.4.9  on kernel 2.6.22.7 I found a bug in file sysctl_net_ipsec.c.
The initialization of ipsec_table is improper for newer kernel versions since ctl_table structure was updated.
The 7th parameter which refer to *parent was initialized mistakenly with *proc_handler as it was in older kernel versions.
As a result ipsec proc entries under /proc/sys/net/ipsec were created as directories instead of files which led  to improper behavior of openswan.
Patch attached - the patch is only for newer kernel versions.Ofcourse it should be updated for backward compatibility.


Regards,
Eran Ben-Avi



__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sysctl.patch --]
[-- Type: text/x-patch; name="sysctl.patch", Size: 3091 bytes --]

--- linux/net/ipsec/sysctl_net_ipsec.c	2004-07-10 17:11:18.000000000 -0200
+++ ../openswan-2.4.9_a/linux/net/ipsec/sysctl_net_ipsec.c	2007-11-06 11:13:13.000000000 -0200
@@ -74,45 +74,45 @@
 static ctl_table ipsec_table[] = {
 #ifdef CONFIG_KLIPS_DEBUG
 	{ NET_IPSEC_DEBUG_AH, "debug_ah", &debug_ah,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_ESP, "debug_esp", &debug_esp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_TUNNEL, "debug_tunnel", &debug_tunnel,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_EROUTE, "debug_eroute", &debug_eroute,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_SPI, "debug_spi", &debug_spi,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_RADIJ, "debug_radij", &debug_radij,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_NETLINK, "debug_netlink", &debug_netlink,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_XFORM, "debug_xform", &debug_xform,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_RCV, "debug_rcv", &debug_rcv,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_PFKEY, "debug_pfkey", &debug_pfkey,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_DEBUG_VERBOSE, "debug_verbose",&sysctl_ipsec_debug_verbose,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 #ifdef CONFIG_KLIPS_IPCOMP
 	{ NET_IPSEC_DEBUG_IPCOMP, "debug_ipcomp", &sysctl_ipsec_debug_ipcomp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 #endif /* CONFIG_KLIPS_IPCOMP */
 
 #ifdef CONFIG_KLIPS_REGRESS
 	{ NET_IPSEC_REGRESS_PFKEY_LOSSAGE, "pfkey_lossage",
 	  &sysctl_ipsec_regress_pfkey_lossage,
-	  sizeof(int), 0644, NULL, &proc_dointvec},
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},
 #endif /* CONFIG_KLIPS_REGRESS */
 
 #endif /* CONFIG_KLIPS_DEBUG */
 	{ NET_IPSEC_ICMP, "icmp", &sysctl_ipsec_icmp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_INBOUND_POLICY_CHECK, "inbound_policy_check", &sysctl_ipsec_inbound_policy_check,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{ NET_IPSEC_TOS, "tos", &sysctl_ipsec_tos,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
 	{0}
 };
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: improper initialization of ipsec_table  in openswan 2.4.9
  2007-11-06 12:04 improper initialization of ipsec_table in openswan 2.4.9 Eran Ben-Avi
@ 2007-11-08  0:29 ` David McCullough
  2007-11-08  3:18   ` [Openswan dev] " Paul Wouters
  0 siblings, 1 reply; 3+ messages in thread
From: David McCullough @ 2007-11-08  0:29 UTC (permalink / raw)
  To: Eran Ben-Avi; +Cc: linux-crypto, dev

[-- Attachment #1: Type: text/plain, Size: 1750 bytes --]


Jivin Eran Ben-Avi lays it down ...
> Hi,
> 
> While working with openswan 2.4.9  on kernel 2.6.22.7 I found a bug in file sysctl_net_ipsec.c.
> The initialization of ipsec_table is improper for newer kernel versions since ctl_table structure was updated.
> The 7th parameter which refer to *parent was initialized mistakenly with *proc_handler as it was in older kernel versions.
> As a result ipsec proc entries under /proc/sys/net/ipsec were created as directories instead of files which led  to improper behavior of openswan.
> Patch attached - the patch is only for newer kernel versions.Ofcourse it should be updated for backward compatibility.

The newly released 2.4.10 version has a fix for this included,  but it
only names the one field to avoid the structure alignment changes.
Probably more a topic for the openswan dev list.

I figure it needs to be more like the attached changes to 2.4.9 going forward.
Will repost one I update to 2.4.10 (and then probably 2.5...) ;-)

Cheers,
Davidm

> --- linux/net/ipsec/sysctl_net_ipsec.c	2004-07-10 17:11:18.000000000 -0200
> +++ ../openswan-2.4.9_a/linux/net/ipsec/sysctl_net_ipsec.c	2007-11-06 11:13:13.000000000 -0200
> @@ -74,45 +74,45 @@
>  static ctl_table ipsec_table[] = {
>  #ifdef CONFIG_KLIPS_DEBUG
>  	{ NET_IPSEC_DEBUG_AH, "debug_ah", &debug_ah,
> -	  sizeof(int), 0644, NULL, &proc_dointvec},    
> +	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
>  	{ NET_IPSEC_DEBUG_ESP, "debug_esp", &debug_esp,
> -	  sizeof(int), 0644, NULL, &proc_dointvec},    
> +	  sizeof(int), 0644, NULL, NULL, &proc_dointvec},    
....


-- 
David McCullough,  david_mccullough@securecomputing.com,   Ph:+61 734352815
Secure Computing - SnapGear  http://www.uCdot.org http://www.cyberguard.com

[-- Attachment #2: sysctl.diff --]
[-- Type: text/x-diff, Size: 6495 bytes --]

Index: openswan/linux/net/ipsec/sysctl_net_ipsec.c
===================================================================
RCS file: /cvs/sw/new-wave/openswan/linux/net/ipsec/sysctl_net_ipsec.c,v
retrieving revision 1.2
diff -u -r1.2 sysctl_net_ipsec.c
--- openswan/linux/net/ipsec/sysctl_net_ipsec.c	26 Apr 2007 13:21:26 -0000	1.2
+++ openswan/linux/net/ipsec/sysctl_net_ipsec.c	8 Nov 2007 00:21:08 -0000
@@ -73,58 +73,171 @@
 };
 
 static ctl_table ipsec_table[] = {
+
 #ifdef CONFIG_KLIPS_DEBUG
-	{ NET_IPSEC_DEBUG_AH, "debug_ah", &debug_ah,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_ESP, "debug_esp", &debug_esp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_TUNNEL, "debug_tunnel", &debug_tunnel,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_EROUTE, "debug_eroute", &debug_eroute,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_SPI, "debug_spi", &debug_spi,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_RADIJ, "debug_radij", &debug_radij,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_NETLINK, "debug_netlink", &debug_netlink,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_XFORM, "debug_xform", &debug_xform,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_RCV, "debug_rcv", &debug_rcv,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_PFKEY, "debug_pfkey", &debug_pfkey,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_DEBUG_VERBOSE, "debug_verbose",&sysctl_ipsec_debug_verbose,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_AH,
+		.procname     = "debug_ah",
+		.data         = &debug_ah,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_ESP,
+		.procname     = "debug_esp",
+		.data         = &debug_esp,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_TUNNEL,
+		.procname     = "debug_tunnel",
+		.data         = &debug_tunnel,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_EROUTE,
+		.procname     = "debug_eroute",
+		.data         = &debug_eroute,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_SPI,
+		.procname     = "debug_spi",
+		.data         = &debug_spi,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_RADIJ,
+		.procname     = "debug_radij",
+		.data         = &debug_radij,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_NETLINK,
+		.procname     = "debug_netlink",
+		.data         = &debug_netlink,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_XFORM,
+		.procname     = "debug_xform",
+		.data         = &debug_xform,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_RCV,
+		.procname     = "debug_rcv",
+		.data         = &debug_rcv,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_PFKEY,
+		.procname     = "debug_pfkey",
+		.data         = &debug_pfkey,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_VERBOSE,
+		.procname     = "debug_verbose",
+		.data         = &sysctl_ipsec_debug_verbose,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
 #ifdef CONFIG_KLIPS_IPCOMP
-	{ NET_IPSEC_DEBUG_IPCOMP, "debug_ipcomp", &sysctl_ipsec_debug_ipcomp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
+	{
+		.ctl_name     = NET_IPSEC_DEBUG_IPCOMP,
+		.procname     = "debug_ipcomp",
+		.data         = &sysctl_ipsec_debug_ipcomp,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
 #endif /* CONFIG_KLIPS_IPCOMP */
 
 #ifdef CONFIG_KLIPS_REGRESS
-	{ NET_IPSEC_REGRESS_PFKEY_LOSSAGE, "pfkey_lossage",
-	  &sysctl_ipsec_regress_pfkey_lossage,
-	  sizeof(int), 0644, NULL, &proc_dointvec},
+	{
+		.ctl_name     = NET_IPSEC_REGRESS_PFKEY_LOSSAGE,
+		.procname     = "pfkey_lossage",
+		.data         = &sysctl_ipsec_regress_pfkey_lossage,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
 #endif /* CONFIG_KLIPS_REGRESS */
 
 #endif /* CONFIG_KLIPS_DEBUG */
-	{ NET_IPSEC_ICMP, "icmp", &sysctl_ipsec_icmp,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_INBOUND_POLICY_CHECK, "inbound_policy_check", &sysctl_ipsec_inbound_policy_check,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{ NET_IPSEC_TOS, "tos", &sysctl_ipsec_tos,
-	  sizeof(int), 0644, NULL, &proc_dointvec},    
-	{0}
+
+	{
+		.ctl_name     = NET_IPSEC_ICMP,
+		.procname     = "icmp",
+		.data         = &sysctl_ipsec_icmp,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_INBOUND_POLICY_CHECK,
+		.procname     = "inbound_policy_check",
+		.data         = &sysctl_ipsec_inbound_policy_check,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = NET_IPSEC_TOS,
+		.procname     = "tos",
+		.data         = &sysctl_ipsec_tos,
+		.maxlen       = sizeof(int),
+		.mode         = 0644,
+		.proc_handler = &proc_dointvec
+	},
+	{
+		.ctl_name     = 0
+	}
 };
 
 static ctl_table ipsec_net_table[] = {
-        { NET_IPSEC, "ipsec", NULL, 0, 0555, ipsec_table },
-        { 0 }
+	{
+		.ctl_name     = NET_IPSEC,
+		.procname     = "ipsec",
+		.mode         = 0555,
+		.child        = ipsec_table
+	},
+	{
+		.ctl_name     = 0
+	}
 };
  
 static ctl_table ipsec_root_table[] = {
-        { CTL_NET, "net", NULL, 0, 0555, ipsec_net_table },
-        { 0 }
+	{
+		.ctl_name     = CTL_NET,
+		.procname     = "net",
+		.mode         = 0555,
+		.child        = ipsec_net_table
+	},
+	{
+		.ctl_name     = 0
+	}
 };
  
 static struct ctl_table_header *ipsec_table_header;

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Openswan dev] improper initialization of ipsec_table in openswan 2.4.9
  2007-11-08  0:29 ` David McCullough
@ 2007-11-08  3:18   ` Paul Wouters
  0 siblings, 0 replies; 3+ messages in thread
From: Paul Wouters @ 2007-11-08  3:18 UTC (permalink / raw)
  To: David McCullough; +Cc: dev, linux-crypto, Eran Ben-Avi

On Thu, 8 Nov 2007, David McCullough wrote:

> Date: Thu, 8 Nov 2007 10:29:13 +1000
> From: David McCullough <David_Mccullough@securecomputing.com>
> Cc:  <dev@lists.openswan.org>,  <linux-crypto@vger.kernel.org>
> To: Eran Ben-Avi <eranpublic@yahoo.com>
> Subject: Re: [Openswan dev] improper initialization of ipsec_table in openswan
>      2.4.9

> The newly released 2.4.10 version has a fix for this included,  but it
> only names the one field to avoid the structure alignment changes.
> Probably more a topic for the openswan dev list.
>
> I figure it needs to be more like the attached changes to 2.4.9 going forward.
> Will repost one I update to 2.4.10 (and then probably 2.5...) ;-)

The fix as attached, naming the structure items, is already in #testing (2.5.x)

Paul

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-11-08  3:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-06 12:04 improper initialization of ipsec_table in openswan 2.4.9 Eran Ben-Avi
2007-11-08  0:29 ` David McCullough
2007-11-08  3:18   ` [Openswan dev] " Paul Wouters

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).