Skip to contentSkip to conversation

: Fix invalid return values from sctp_new

PATCH In review Patrick McHardy 2 participants 5 messages Sep 20, 2004 netfilter

lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

PATCH 1/1 : Fix invalid return values from sctp_new

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

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2004/09/20 10:06:40+02:00 [email protected]
# [NETFILTER]: Fix invalid return values from sctp_new
#
# Signed-off-by: Patrick McHardy <[email protected]>
#

# net/ipv4/netfilter/ip_conntrack_proto_sctp.c
#   2004/09/20 10:06:16+02:00 [email protected] +5 -5
#   [NETFILTER]: Fix invalid return values from sctp_new
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
# 
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

#1 lore raw
Quoted 11 lines · Patrick McHardy

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;

Signature · 2 lines
Regards,
Kiran Kumar Immidi
author
#2 lore raw
Quoted 4 lines · Kiran Kumar Immidi

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