buffer overflow in ip_ct_{ftp,tftp,irc}

buffer overflow in ip_ct_{ftp,tftp,irc}

[Samir Bellabes]

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

Hi,

when loading ip_conntrack_{ftp,tftp,irc} with 'ports=1234567890'
parameter option for example, a buffer overflow occur when :
sprintf(tmpname, "ftp-%d", ports[i]);
because of sizeof("ftp-1234567890") > 10
10 is the size of each array *_names[port][10]

Please apply this patch.

regards, 
Samir Bellabes


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nf_bad_param_port.patch --]
[-- Type: text/x-patch, Size: 2832 bytes --]

tree f8f9de37b0294e8049a959dfa5acc2efc64ab231
parent 48bc41a49c4f3aa760dff84e7f71437f5ed520fe
author Samir Bellabes <sbellabes@mandriva.com> 1126118922 +0200
committer Samir Bellabes <sbellabes@mandriva.com> 1126118922 +0200

[NETFILTER] Check for bad parameter value of 'ports' in ip_ct_{ftp,tftp,irc}

A buffer overflow occur when parameter 'ports' value is > 65535 :
if the number of digits of ports exceeds, for the ftp example:
sizeof ftp_names[A_PORT][10] - strlen("ftp-") = 10-4 = 6 
then : sprintf(tmpname, "ftp-%d", ports[i]) produce the overflow.

This patch checks for 0 < port < 65356.

Signed-off-by: Samir Bellabes <sbellabes@mandriva.com>
------------------------------------------------------------------------------

 ip_conntrack_ftp.c  |    7 +++++++
 ip_conntrack_irc.c  |    7 +++++++
 ip_conntrack_tftp.c |    7 +++++++
 3 files changed, 21 insertions(+)

------------------------------------------------------------------------------

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -478,6 +478,13 @@ static int __init init(void)
 		ports[ports_c++] = FTP_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING "ip_ct_ftp: ERROR port"
+			       "should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		ftp[i].tuple.src.u.tcp.port = htons(ports[i]);
 		ftp[i].tuple.dst.protonum = IPPROTO_TCP;
 		ftp[i].mask.src.u.tcp.port = 0xFFFF;
diff --git a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
--- a/net/ipv4/netfilter/ip_conntrack_irc.c
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -268,6 +268,13 @@ static int __init init(void)
 		ports[ports_c++] = IRC_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING "ip_conntrack_irc: ERROR port"
+			       "should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		hlpr = &irc_helpers[i];
 		hlpr->tuple.src.u.tcp.port = htons(ports[i]);
 		hlpr->tuple.dst.protonum = IPPROTO_TCP;
diff --git a/net/ipv4/netfilter/ip_conntrack_tftp.c b/net/ipv4/netfilter/ip_conntrack_tftp.c
--- a/net/ipv4/netfilter/ip_conntrack_tftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_tftp.c
@@ -122,6 +122,13 @@ static int __init init(void)
 		ports[ports_c++] = TFTP_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING
+			       "ERROR port should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		/* Create helper structure */
 		memset(&tftp[i], 0, sizeof(struct ip_conntrack_helper));
 

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Samir Bellabes]

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

Samir Bellabes <sbellabes@mandriva.com> writes:

> when loading ip_conntrack_{ftp,tftp,irc} with 'ports=1234567890'
> parameter option for example, a buffer overflow occur when :
> sprintf(tmpname, "ftp-%d", ports[i]);
> because of sizeof("ftp-1234567890") > 10
> 10 is the size of each array *_names[port][10]

Resending patch with a better changelog.
Sorry for the noise.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nf_bad_param_port.patch --]
[-- Type: text/x-patch, Size: 2854 bytes --]

tree f8f9de37b0294e8049a959dfa5acc2efc64ab231
parent 48bc41a49c4f3aa760dff84e7f71437f5ed520fe
author Samir Bellabes <sbellabes@mandriva.com> 1126118922 +0200
committer Samir Bellabes <sbellabes@mandriva.com> 1126118922 +0200

[NETFILTER] Check for bad parameter value of 'ports' in ip_ct_{ftp,tftp,irc}

A buffer overflow occur when parameter 'ports' value is > 65535 :
if the number of digits of ports exceeds, for the ftp example:
10 - strlen("ftp-")  = 6 
then : sprintf(tmpname, "ftp-%d", ports[i]) produce the overflow.
10 is the size of each char array *_names[A_PORT][10]

This patch checks for 0 < port < 65536.

Signed-off-by: Samir Bellabes <sbellabes@mandriva.com>
------------------------------------------------------------------------------

 ip_conntrack_ftp.c  |    7 +++++++
 ip_conntrack_irc.c  |    7 +++++++
 ip_conntrack_tftp.c |    7 +++++++
 3 files changed, 21 insertions(+)

------------------------------------------------------------------------------

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -478,6 +478,13 @@ static int __init init(void)
 		ports[ports_c++] = FTP_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING "ip_ct_ftp: ERROR port"
+			       "should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		ftp[i].tuple.src.u.tcp.port = htons(ports[i]);
 		ftp[i].tuple.dst.protonum = IPPROTO_TCP;
 		ftp[i].mask.src.u.tcp.port = 0xFFFF;
diff --git a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
--- a/net/ipv4/netfilter/ip_conntrack_irc.c
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -268,6 +268,13 @@ static int __init init(void)
 		ports[ports_c++] = IRC_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING "ip_conntrack_irc: ERROR port"
+			       "should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		hlpr = &irc_helpers[i];
 		hlpr->tuple.src.u.tcp.port = htons(ports[i]);
 		hlpr->tuple.dst.protonum = IPPROTO_TCP;
diff --git a/net/ipv4/netfilter/ip_conntrack_tftp.c b/net/ipv4/netfilter/ip_conntrack_tftp.c
--- a/net/ipv4/netfilter/ip_conntrack_tftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_tftp.c
@@ -122,6 +122,13 @@ static int __init init(void)
 		ports[ports_c++] = TFTP_PORT;
 
 	for (i = 0; i < ports_c; i++) {
+		/* don't allow bad port values */
+		if (ports[i] < 1 || ports[i] > 65535) {
+			printk(KERN_WARNING
+			       "ERROR port should be between 1 and 65535\n");
+			fini();
+			return -EINVAL;
+		}
 		/* Create helper structure */
 		memset(&tftp[i], 0, sizeof(struct ip_conntrack_helper));
 

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Pablo Neira]

Samir Bellabes wrote:
>  	for (i = 0; i < ports_c; i++) {
> +		/* don't allow bad port values */
> +		if (ports[i] < 1 || ports[i] > 65535) {
> +			printk(KERN_WARNING "ip_ct_ftp: ERROR port"
> +			       "should be between 1 and 65535\n");
> +			fini();
> +			return -EINVAL;
> +		}

Better something like this?
 
                                      -static int ports[MAX_PORTS]; 
                                               +static short 
ports[MAX_PORTS]; 
static int ports_c; 
        -module_param_array(ports, int, &ports_c, 0400); 
                 +module_param_array(ports, short, &ports_c, 0400); 
 


--
Pablo

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Pablo Neira]

Pablo Neira wrote:
> Samir Bellabes wrote:
> 
>>      for (i = 0; i < ports_c; i++) {
>> +        /* don't allow bad port values */
>> +        if (ports[i] < 1 || ports[i] > 65535) {
>> +            printk(KERN_WARNING "ip_ct_ftp: ERROR port"
>> +                   "should be between 1 and 65535\n");
>> +            fini();
>> +            return -EINVAL;
>> +        }
> 
> 
> Better something like this?

Damn, sorry, my mail client has mangled the email, I meant:

-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
static int ports_c;
-module_param_array(ports,int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);

--
Pablo

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Patrick McHardy]

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

Pablo Neira wrote:
> Pablo Neira wrote:
> 
>> Samir Bellabes wrote:
>>
>>>      for (i = 0; i < ports_c; i++) {
>>> +        /* don't allow bad port values */
>>> +        if (ports[i] < 1 || ports[i] > 65535) {
>>> +            printk(KERN_WARNING "ip_ct_ftp: ERROR port"
>>> +                   "should be between 1 and 65535\n");
>>> +            fini();
>>> +            return -EINVAL;
>>> +        }
>>
>>
>>
>> Better something like this?
> 
> Damn, sorry, my mail client has mangled the email, I meant:
> 
> -static int ports[MAX_PORTS];
> +static short ports[MAX_PORTS];
> static int ports_c;
> -module_param_array(ports,int, &ports_c, 0400);
> +module_param_array(ports, short, &ports_c, 0400);

I agree, I've applied this patch instead. Thanks.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3432 bytes --]

[NETFILTER]: Use correct types for "ports" module parameter

With large port numbers the helper_names buffer can overflow.
Noticed by Samir Bellabes <sbellabes@mandriva.com>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit d53f0d343998b81945723c43046c4f2ee301e45b
tree 2e8a7c30c3fb32cae0eacb4231ac3554e18f6a47
parent 1d8674edb534a3c5cb549bfde5a39fa5598cb3bc
author Patrick McHardy <kaber@trash.net> Sat, 10 Sep 2005 00:58:11 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 10 Sep 2005 00:58:11 +0200

 net/ipv4/netfilter/ip_conntrack_ftp.c  |    6 +++---
 net/ipv4/netfilter/ip_conntrack_irc.c  |    6 +++---
 net/ipv4/netfilter/ip_conntrack_tftp.c |    6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -29,9 +29,9 @@ static char *ftp_buffer;
 static DEFINE_SPINLOCK(ip_ftp_lock);
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 
 static int loose;
 module_param(loose, int, 0600);
@@ -450,7 +450,7 @@ out_update_nl:
 }
 
 static struct ip_conntrack_helper ftp[MAX_PORTS];
-static char ftp_names[MAX_PORTS][10];
+static char ftp_names[MAX_PORTS][sizeof("ftp-65535")];
 
 /* Not __exit: called from init() */
 static void fini(void)
diff --git a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
--- a/net/ipv4/netfilter/ip_conntrack_irc.c
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -34,7 +34,7 @@
 #include <linux/moduleparam.h>
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
 static int max_dcc_channels = 8;
 static unsigned int dcc_timeout = 300;
@@ -52,7 +52,7 @@ EXPORT_SYMBOL_GPL(ip_nat_irc_hook);
 MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
 MODULE_DESCRIPTION("IRC (DCC) connection tracking helper");
 MODULE_LICENSE("GPL");
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 MODULE_PARM_DESC(ports, "port numbers of IRC servers");
 module_param(max_dcc_channels, int, 0400);
 MODULE_PARM_DESC(max_dcc_channels, "max number of expected DCC channels per IRC session");
@@ -240,7 +240,7 @@ static int help(struct sk_buff **pskb,
 }
 
 static struct ip_conntrack_helper irc_helpers[MAX_PORTS];
-static char irc_names[MAX_PORTS][10];
+static char irc_names[MAX_PORTS][sizeof("irc-65535")];
 
 static void fini(void);
 
diff --git a/net/ipv4/netfilter/ip_conntrack_tftp.c b/net/ipv4/netfilter/ip_conntrack_tftp.c
--- a/net/ipv4/netfilter/ip_conntrack_tftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_tftp.c
@@ -26,9 +26,9 @@ MODULE_DESCRIPTION("tftp connection trac
 MODULE_LICENSE("GPL");
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 MODULE_PARM_DESC(ports, "port numbers of tftp servers");
 
 #if 0
@@ -100,7 +100,7 @@ static int tftp_help(struct sk_buff **ps
 }
 
 static struct ip_conntrack_helper tftp[MAX_PORTS];
-static char tftp_names[MAX_PORTS][10];
+static char tftp_names[MAX_PORTS][sizeof("tftp-65535")];
 
 static void fini(void)
 {

[PATCH] Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Harald Welte]

[-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --]

Dave, please consider the appended patch.

On Thu, Sep 08, 2005 at 01:48:08AM +0200, Pablo Neira wrote:
> Pablo Neira wrote:
> >Samir Bellabes wrote:
> >>     for (i = 0; i < ports_c; i++) {
> >>+        /* don't allow bad port values */
> >>+        if (ports[i] < 1 || ports[i] > 65535) {
> >>+            printk(KERN_WARNING "ip_ct_ftp: ERROR port"
> >>+                   "should be between 1 and 65535\n");
> >>+            fini();
> >>+            return -EINVAL;
> >>+        }
> >Better something like this?
> 
> Damn, sorry, my mail client has mangled the email, I meant:
> 
> -static int ports[MAX_PORTS];
> +static short ports[MAX_PORTS];
> static int ports_c;
> -module_param_array(ports,int, &ports_c, 0400);
> +module_param_array(ports, short, &ports_c, 0400);

I agree, it looks cleaner than the explicit checks.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #1.2: 54-ports-short.patch --]
[-- Type: text/plain, Size: 2706 bytes --]

[NETFILTER] Don't allow port numbers > 65535

This patch is the result of comments from Samir Bellabas and Pablo Neira.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 707a1ddcc1aed83c3c5283925635d6a8982363f1
tree 62fdc65b0c508f43e0182939274ea6b6848d6f2d
parent 49f4a3e845089f9d90e7b481e1bb72ab217d69f5
author Harald Welte <laforge@netfilter.org> Sa, 10 Sep 2005 09:37:22 +0200
committer Harald Welte <laforge@netfilter.org> Sa, 10 Sep 2005 09:37:22 +0200

 net/ipv4/netfilter/ip_conntrack_ftp.c  |    4 ++--
 net/ipv4/netfilter/ip_conntrack_irc.c  |    4 ++--
 net/ipv4/netfilter/ip_conntrack_tftp.c |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -29,9 +29,9 @@ static char *ftp_buffer;
 static DEFINE_SPINLOCK(ip_ftp_lock);
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 
 static int loose;
 module_param(loose, int, 0600);
diff --git a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
--- a/net/ipv4/netfilter/ip_conntrack_irc.c
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -34,7 +34,7 @@
 #include <linux/moduleparam.h>
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
 static int max_dcc_channels = 8;
 static unsigned int dcc_timeout = 300;
@@ -52,7 +52,7 @@ EXPORT_SYMBOL_GPL(ip_nat_irc_hook);
 MODULE_AUTHOR("Harald Welte <laforge@netfilter.org>");
 MODULE_DESCRIPTION("IRC (DCC) connection tracking helper");
 MODULE_LICENSE("GPL");
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 MODULE_PARM_DESC(ports, "port numbers of IRC servers");
 module_param(max_dcc_channels, int, 0400);
 MODULE_PARM_DESC(max_dcc_channels, "max number of expected DCC channels per IRC session");
diff --git a/net/ipv4/netfilter/ip_conntrack_tftp.c b/net/ipv4/netfilter/ip_conntrack_tftp.c
--- a/net/ipv4/netfilter/ip_conntrack_tftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_tftp.c
@@ -26,9 +26,9 @@ MODULE_DESCRIPTION("tftp connection trac
 MODULE_LICENSE("GPL");
 
 #define MAX_PORTS 8
-static int ports[MAX_PORTS];
+static short ports[MAX_PORTS];
 static int ports_c;
-module_param_array(ports, int, &ports_c, 0400);
+module_param_array(ports, short, &ports_c, 0400);
 MODULE_PARM_DESC(ports, "port numbers of tftp servers");
 
 #if 0

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Amin Azez]

Patrick McHardy wrote:
...

> I agree, I've applied this patch instead. Thanks.

Dumb question follows:

What has this patch been applied to?...

pom-ng?
a git tree somewhere?
and svn kernel tree somewhere?

Thanks

Azez

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Patrick McHardy]

Amin Azez wrote:
> Patrick McHardy wrote:
>
>>I agree, I've applied this patch instead. Thanks.
> 
> Dumb question follows:
> 
> What has this patch been applied to?...
> 
> pom-ng?
> a git tree somewhere?
> and svn kernel tree somewhere?

My personal git tree. I usually push it to Dave every week
or so if it contains patches.

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Yasuyuki KOZAKAI]

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 10 Sep 2005 00:59:49 +0200

> > -static int ports[MAX_PORTS];
> > +static short ports[MAX_PORTS];
> > static int ports_c;
> > -module_param_array(ports,int, &ports_c, 0400);
> > +module_param_array(ports, short, &ports_c, 0400);
> 
> I agree, I've applied this patch instead. Thanks.

Why don't you use u_int16_t and ushort instead of short ?

-----------------------------------------------------------------
Yasuyuki Kozakai @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Pablo Neira]

Yasuyuki KOZAKAI wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Sat, 10 Sep 2005 00:59:49 +0200
> 
> 
>>>-static int ports[MAX_PORTS];
>>>+static short ports[MAX_PORTS];
>>>static int ports_c;
>>>-module_param_array(ports,int, &ports_c, 0400);
>>>+module_param_array(ports, short, &ports_c, 0400);
>>
>>I agree, I've applied this patch instead. Thanks.
> 
> 
> Why don't you use u_int16_t and ushort instead of short ?

Short reply, because the u_int16_t type isn't defined in moduleparam.h. 
See the param_set_* stuff.

--
Pablo

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Harald Welte]

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

On Tue, Sep 20, 2005 at 04:11:57PM +0900, Yasuyuki KOZAKAI wrote:
> 
> From: Patrick McHardy <kaber@trash.net>
> Date: Sat, 10 Sep 2005 00:59:49 +0200
> 
> > > -static int ports[MAX_PORTS];
> > > +static short ports[MAX_PORTS];
> > > static int ports_c;
> > > -module_param_array(ports,int, &ports_c, 0400);
> > > +module_param_array(ports, short, &ports_c, 0400);
> > 
> > I agree, I've applied this patch instead. Thanks.
> 
> Why don't you use u_int16_t and ushort instead of short ?

I tried this first, but the module_parm_* macros only deal with short.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Yasuyuki KOZAKAI]

[-- Attachment #1: Type: Text/Plain, Size: 1080 bytes --]

From: Harald Welte <laforge@netfilter.org>
Date: Tue, 20 Sep 2005 11:35:10 +0200

> On Tue, Sep 20, 2005 at 04:11:57PM +0900, Yasuyuki KOZAKAI wrote:
> > 
> > From: Patrick McHardy <kaber@trash.net>
> > Date: Sat, 10 Sep 2005 00:59:49 +0200
> > 
> > > > -static int ports[MAX_PORTS];
> > > > +static short ports[MAX_PORTS];
> > > > static int ports_c;
> > > > -module_param_array(ports,int, &ports_c, 0400);
> > > > +module_param_array(ports, short, &ports_c, 0400);
> > > 
> > > I agree, I've applied this patch instead. Thanks.
> > 
> > Why don't you use u_int16_t and ushort instead of short ?
> 
> I tried this first, but the module_parm_* macros only deal with short.

Really ? At least, the current moduleparam.h in David's git tree includes
declarations for param_*_ushort(). And I succeeded to compile and use ftp
module with the attached patch.

Signed-off-by: Yasuyuki Kozakai <yasuyuki.kozakai@toshiba.co.jp>

Which kernel you tried ?

-----------------------------------------------------------------
Yasuyuki Kozakai @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>


[-- Attachment #2: ushort-ftp-ports.patch --]
[-- Type: Text/Plain, Size: 529 bytes --]

diff --git a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -29,9 +29,9 @@ static char *ftp_buffer;
 static DEFINE_SPINLOCK(ip_ftp_lock);
 
 #define MAX_PORTS 8
-static short ports[MAX_PORTS];
+static u_int16_t ports[MAX_PORTS];
 static int ports_c;
-module_param_array(ports, short, &ports_c, 0400);
+module_param_array(ports, ushort, &ports_c, 0400);
 
 static int loose;
 module_param(loose, int, 0600);

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Harald Welte]

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

On Tue, Sep 20, 2005 at 09:48:21PM +0900, Yasuyuki KOZAKAI wrote:

> Really ? At least, the current moduleparam.h in David's git tree includes
> declarations for param_*_ushort(). And I succeeded to compile and use ftp
> module with the attached patch.

ok "ushort" will work, but not "u_int16_t".  I'll prepare a patch for
all helpers.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: buffer overflow in ip_ct_{ftp,tftp,irc}

[Yasuyuki KOZAKAI]

From: Harald Welte <laforge@netfilter.org>
Date: Tue, 20 Sep 2005 16:15:20 +0200

> On Tue, Sep 20, 2005 at 09:48:21PM +0900, Yasuyuki KOZAKAI wrote:
> 
> > Really ? At least, the current moduleparam.h in David's git tree includes
> > declarations for param_*_ushort(). And I succeeded to compile and use ftp
> > module with the attached patch.
> 
> ok "ushort" will work, but not "u_int16_t".  I'll prepare a patch for
> all helpers.

Sorry for confusing. "using u_int16_t" means just "u_int16_t ports[MAX_PORTS]".
Anyway, thanks for consideration.

-----------------------------------------------------------------
Yasuyuki Kozakai @ USAGI Project <yasuyuki.kozakai@toshiba.co.jp>