[PATCH]: Fix invalid return values from sctp_new

[PATCH]: Fix invalid return values from sctp_new

[Patrick McHardy]

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

Hi Kiran,

there are multiple places in sctp_new where it returns -1, but
it looks like it really wants to return "invalid". The call to
protocol->new in init_conntrack looks like this:

        if (!protocol->new(conntrack, skb)) {
                kmem_cache_free(ip_conntrack_cachep, conntrack);
                return NULL;
        }

so it has to return 0 to say "invalid". Please check if the
attached patch which fixes these places is correct.

Regards
Patrick


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

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/09/20 10:06:40+02:00 kaber@coreworks.de 
#   [NETFILTER]: Fix invalid return values from sctp_new
#   
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
# net/ipv4/netfilter/ip_conntrack_proto_sctp.c
#   2004/09/20 10:06:16+02:00 kaber@coreworks.de +5 -5
#   [NETFILTER]: Fix invalid return values from sctp_new
#   
#   Signed-off-by: Patrick McHardy <kaber@trash.net>
# 
diff -Nru a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-20 10:26:10 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c	2004-09-20 10:26:10 +02:00
@@ -430,16 +430,16 @@
 	DEBUGP("\n");
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4, &sctph, sizeof(sctph)) != 0)
-		return -1;
+		return 0;
 
 	if (do_basic_checks(conntrack, skb, map) != 0)
-		return -1;
+		return 0;
 
 	/* If an OOTB packet has any of these chunks discard (Sec 8.4) */
 	if ((test_bit (SCTP_CID_ABORT, (void *)map))
 		|| (test_bit (SCTP_CID_SHUTDOWN_COMPLETE, (void *)map))
 		|| (test_bit (SCTP_CID_COOKIE_ACK, (void *)map))) {
-		return -1;
+		return 0;
 	}
 
 	newconntrack = SCTP_CONNTRACK_MAX;
@@ -461,7 +461,7 @@
 
 				if (skb_copy_bits(skb, offset + sizeof (sctp_chunkhdr_t), 
 					&inithdr, sizeof(inithdr)) != 0) {
-						return -1;
+						return 0;
 				}
 
 				DEBUGP("Setting vtag %x for new conn\n", 
@@ -471,7 +471,7 @@
 								inithdr.init_tag;
 			} else {
 				/* Sec 8.5.1 (A) */
-				return -1;
+				return 0;
 			}
 		}
 		/* If it is a shutdown ack OOTB packet, we expect a return

Re: [PATCH]: Fix invalid return values from sctp_new

[Kiran Kumar Immidi]

On Monday 20 September 2004 13:58, Patrick McHardy wrote:

> there are multiple places in sctp_new where it returns -1, but
> it looks like it really wants to return "invalid". The call to
> protocol->new in init_conntrack looks like this:
>
>         if (!protocol->new(conntrack, skb)) {
>                 kmem_cache_free(ip_conntrack_cachep, conntrack);
>                 return NULL;
>         }
>
> so it has to return 0 to say "invalid". Please check if the
> attached patch which fixes these places is correct.

  This seems fair. But the place I copied this from 
[ ip_conntrack_proto_tcp.c:tcp_new() ]  also needs to be fixed I guess..

@@ -237,7 +237,7 @@
        struct tcphdr tcph;

        if (skb_copy_bits(skb, skb->nh.iph->ihl * 4, &tcph, sizeof(tcph)) != 
0)
-               return -1;
+               return 0;


-- 
Regards,
Kiran Kumar Immidi

Re: [PATCH]: Fix invalid return values from sctp_new

[Patrick McHardy]

Kiran Kumar Immidi wrote:

>  This seems fair. But the place I copied this from 
>[ ip_conntrack_proto_tcp.c:tcp_new() ]  also needs to be fixed I guess..
>  
>

Thanks. The others are fine, and I can't find an old version of
ip_conntrack_proto_tcp where it returned -1.

Regards
Patrick

Re: [PATCH]: Fix invalid return values from sctp_new

[Kiran Kumar Immidi]

On Monday 20 September 2004 14:32, Patrick McHardy wrote:

> Thanks. The others are fine, and I can't find an old version of
> ip_conntrack_proto_tcp where it returned -1.

http://lxr.linux.no/source/net/ipv4/netfilter/ip_conntrack_proto_tcp.c?v=2.6.8.1#L240

I still find it in 2.6.8.1
-- 
Regards,
Kiran Kumar Immidi

Re: [PATCH]: Fix invalid return values from sctp_new

[Patrick McHardy]

Kiran Kumar Immidi wrote:

>http://lxr.linux.no/source/net/ipv4/netfilter/ip_conntrack_proto_tcp.c?v=2.6.8.1#L240
>
>I still find it in 2.6.8.1
>  
>

You're right. Seems like it was silently fixed during the
skb_header_pointer conversion.

Regards
Patrick