* buffer overflow in ip_ct_{ftp,tftp,irc}
@ 2005-09-07 23:11 Samir Bellabes
2005-09-07 23:15 ` Samir Bellabes
0 siblings, 1 reply; 14+ messages in thread
From: Samir Bellabes @ 2005-09-07 23:11 UTC (permalink / raw)
To: netfilter-devel; +Cc: Harald Welte, David S. Miller
[-- 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));
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-07 23:11 buffer overflow in ip_ct_{ftp,tftp,irc} Samir Bellabes @ 2005-09-07 23:15 ` Samir Bellabes 2005-09-07 23:43 ` Pablo Neira 0 siblings, 1 reply; 14+ messages in thread From: Samir Bellabes @ 2005-09-07 23:15 UTC (permalink / raw) To: Samir Bellabes; +Cc: Harald Welte, netfilter-devel, David S. Miller [-- 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)); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-07 23:15 ` Samir Bellabes @ 2005-09-07 23:43 ` Pablo Neira 2005-09-07 23:48 ` Pablo Neira 0 siblings, 1 reply; 14+ messages in thread From: Pablo Neira @ 2005-09-07 23:43 UTC (permalink / raw) To: Samir Bellabes; +Cc: Harald Welte, netfilter-devel, David S. Miller 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-07 23:43 ` Pablo Neira @ 2005-09-07 23:48 ` Pablo Neira 2005-09-09 22:59 ` Patrick McHardy 2005-09-10 7:38 ` [PATCH] " Harald Welte 0 siblings, 2 replies; 14+ messages in thread From: Pablo Neira @ 2005-09-07 23:48 UTC (permalink / raw) To: Pablo Neira Cc: Harald Welte, netfilter-devel, Samir Bellabes, David S. Miller 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-07 23:48 ` Pablo Neira @ 2005-09-09 22:59 ` Patrick McHardy 2005-09-12 8:44 ` Amin Azez ` (2 more replies) 2005-09-10 7:38 ` [PATCH] " Harald Welte 1 sibling, 3 replies; 14+ messages in thread From: Patrick McHardy @ 2005-09-09 22:59 UTC (permalink / raw) To: Pablo Neira Cc: Harald Welte, netfilter-devel, Samir Bellabes, David S. Miller [-- 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) { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-09 22:59 ` Patrick McHardy @ 2005-09-12 8:44 ` Amin Azez 2005-09-12 8:49 ` Patrick McHardy 2005-09-20 7:11 ` Yasuyuki KOZAKAI [not found] ` <200509200711.j8K7Bw3x002184@toshiba.co.jp> 2 siblings, 1 reply; 14+ messages in thread From: Amin Azez @ 2005-09-12 8:44 UTC (permalink / raw) To: Patrick McHardy Cc: Harald Welte, netfilter-devel, Samir Bellabes, David S. Miller 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-12 8:44 ` Amin Azez @ 2005-09-12 8:49 ` Patrick McHardy 0 siblings, 0 replies; 14+ messages in thread From: Patrick McHardy @ 2005-09-12 8:49 UTC (permalink / raw) To: Amin Azez; +Cc: Harald Welte, netfilter-devel, Samir Bellabes, David S. Miller 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-09 22:59 ` Patrick McHardy 2005-09-12 8:44 ` Amin Azez @ 2005-09-20 7:11 ` Yasuyuki KOZAKAI [not found] ` <200509200711.j8K7Bw3x002184@toshiba.co.jp> 2 siblings, 0 replies; 14+ messages in thread From: Yasuyuki KOZAKAI @ 2005-09-20 7:11 UTC (permalink / raw) To: kaber; +Cc: laforge, netfilter-devel, sbellabes, davem, pablo 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> ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <200509200711.j8K7Bw3x002184@toshiba.co.jp>]
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} [not found] ` <200509200711.j8K7Bw3x002184@toshiba.co.jp> @ 2005-09-20 8:10 ` Pablo Neira 2005-09-20 9:35 ` Harald Welte 1 sibling, 0 replies; 14+ messages in thread From: Pablo Neira @ 2005-09-20 8:10 UTC (permalink / raw) To: Yasuyuki KOZAKAI; +Cc: laforge, netfilter-devel, sbellabes, kaber, davem 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} [not found] ` <200509200711.j8K7Bw3x002184@toshiba.co.jp> 2005-09-20 8:10 ` Pablo Neira @ 2005-09-20 9:35 ` Harald Welte 2005-09-20 12:48 ` Yasuyuki KOZAKAI [not found] ` <200509201248.j8KCmNi9009046@toshiba.co.jp> 1 sibling, 2 replies; 14+ messages in thread From: Harald Welte @ 2005-09-20 9:35 UTC (permalink / raw) To: Yasuyuki KOZAKAI; +Cc: netfilter-devel, sbellabes, kaber, pablo, davem [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-20 9:35 ` Harald Welte @ 2005-09-20 12:48 ` Yasuyuki KOZAKAI [not found] ` <200509201248.j8KCmNi9009046@toshiba.co.jp> 1 sibling, 0 replies; 14+ messages in thread From: Yasuyuki KOZAKAI @ 2005-09-20 12:48 UTC (permalink / raw) To: laforge; +Cc: sbellabes, yasuyuki.kozakai, netfilter-devel, davem, kaber, pablo [-- 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); ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <200509201248.j8KCmNi9009046@toshiba.co.jp>]
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} [not found] ` <200509201248.j8KCmNi9009046@toshiba.co.jp> @ 2005-09-20 14:15 ` Harald Welte 2005-09-24 8:43 ` Yasuyuki KOZAKAI 0 siblings, 1 reply; 14+ messages in thread From: Harald Welte @ 2005-09-20 14:15 UTC (permalink / raw) To: Yasuyuki KOZAKAI; +Cc: netfilter-devel, sbellabes, kaber, pablo, davem [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-20 14:15 ` Harald Welte @ 2005-09-24 8:43 ` Yasuyuki KOZAKAI 0 siblings, 0 replies; 14+ messages in thread From: Yasuyuki KOZAKAI @ 2005-09-24 8:43 UTC (permalink / raw) To: laforge; +Cc: sbellabes, yasuyuki.kozakai, netfilter-devel, davem, kaber, pablo 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> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Re: buffer overflow in ip_ct_{ftp,tftp,irc} 2005-09-07 23:48 ` Pablo Neira 2005-09-09 22:59 ` Patrick McHardy @ 2005-09-10 7:38 ` Harald Welte 1 sibling, 0 replies; 14+ messages in thread From: Harald Welte @ 2005-09-10 7:38 UTC (permalink / raw) To: Pablo Neira; +Cc: netfilter-devel, Samir Bellabes, David S. Miller [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2005-09-24 8:43 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-09-07 23:11 buffer overflow in ip_ct_{ftp,tftp,irc} Samir Bellabes
2005-09-07 23:15 ` Samir Bellabes
2005-09-07 23:43 ` Pablo Neira
2005-09-07 23:48 ` Pablo Neira
2005-09-09 22:59 ` Patrick McHardy
2005-09-12 8:44 ` Amin Azez
2005-09-12 8:49 ` Patrick McHardy
2005-09-20 7:11 ` Yasuyuki KOZAKAI
[not found] ` <200509200711.j8K7Bw3x002184@toshiba.co.jp>
2005-09-20 8:10 ` Pablo Neira
2005-09-20 9:35 ` Harald Welte
2005-09-20 12:48 ` Yasuyuki KOZAKAI
[not found] ` <200509201248.j8KCmNi9009046@toshiba.co.jp>
2005-09-20 14:15 ` Harald Welte
2005-09-24 8:43 ` Yasuyuki KOZAKAI
2005-09-10 7:38 ` [PATCH] " Harald Welte
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.