[PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[Brad Chapman]

Hi everyone!

gcc fails with the following error when compiling ip_conntrack_core.c on 2.4.22-ac2
with gcc 3.2.2:

make -C ipv4/netfilter modules
make[2]: Entering directory `/opt/src/linux-2.4.22-ac2+nfpom/net/ipv4/netfilter'
gcc -D__KERNEL__ -I/opt/src/linux-2.4.22-ac2+nfpom/include -Wall -Wstrict-prototypes
-Wno-trigraphs -O2 -fno-strict-aliasing -fno-common -fomit-frame-pointer -pipe
-mpreferred-stack-boundary=2 -march=pentium4 -DMODULE  -nostdinc -iwithprefix
include -DKBUILD_BASENAME=ip_conntrack_core  -c -o ip_conntrack_core.o
ip_conntrack_core.c
ip_conntrack_core.c: In function `getorigdst':
ip_conntrack_core.c:1419: invalid type argument of `->'
ip_conntrack_core.c:1419: invalid type argument of `->'
make[2]: *** [ip_conntrack_core.o] Error 1
make[2]: Leaving directory `/opt/src/linux-2.4.22-ac2+nfpom/net/ipv4/netfilter'
make[1]: *** [_modsubdir_ipv4/netfilter] Error 2
make[1]: Leaving directory `/opt/src/linux-2.4.22-ac2+nfpom/net'
make: *** [_mod_net] Error 2

So I made the following patch to fix it:

--- net/ipv4/netfilter/ip_conntrack_core.c.orig	2003-09-10 14:24:33.000000000 -0400
+++ net/ipv4/netfilter/ip_conntrack_core.c	2003-09-10 14:24:46.000000000 -0400
@@ -1414,14 +1414,14 @@
 getorigdst(struct sock *sk, int optval, void *user, int *len)
 {
 	struct ip_conntrack_tuple_hash *h;
-	struct ip_conntrack_tuple tuple;
+	struct ip_conntrack_tuple *tuple;
 
-	IP_CT_TUPLE_BLANK(&tuple);
-	tuple.src.ip = sk->rcv_saddr;
-	tuple.src.u.tcp.port = sk->sport;
-	tuple.dst.ip = sk->daddr;
-	tuple.dst.u.tcp.port = sk->dport;
-	tuple.dst.protonum = IPPROTO_TCP;
+	IP_CT_TUPLE_BLANK(tuple);
+	tuple->src.ip = sk->rcv_saddr;
+	tuple->src.u.tcp.port = sk->sport;
+	tuple->dst.ip = sk->daddr;
+	tuple->dst.u.tcp.port = sk->dport;
+	tuple->dst.protonum = IPPROTO_TCP;
 
 	/* We only do TCP at the moment: is there a better way? */
 	if (strcmp(sk->prot->name, "TCP") != 0) {
@@ -1435,7 +1435,7 @@
 		return -EINVAL;
 	}
 
-	h = ip_conntrack_find_get(&tuple, NULL);
+	h = ip_conntrack_find_get(tuple, NULL);
 	if (h) {
 		struct sockaddr_in sin;

gcc 3.2.2 compiles this properly with only a warning about the possiblity that
tuple will be uninitialized. The original code looks perfectly legal and proper,
so why would gcc 3.x fail?

NOTE: I did apply some patches from patch-o-matic-20030909, but no conntrack-core-
modifying patches that weren't already present.

TIA

Brad Chapman

=====
Brad Chapman

Permanent e-mail: kakadu_croc@yahoo.com

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com

Re: [PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[Henrik Nordstrom]

On Wed, 10 Sep 2003, Brad Chapman wrote:

> Hi everyone!
> 
> gcc fails with the following error when compiling ip_conntrack_core.c on 2.4.22-ac2
> with gcc 3.2.2:

See my post a few days ago..

> +	struct ip_conntrack_tuple *tuple;

> +	IP_CT_TUPLE_BLANK(tuple);

Won't work.. the tuple needs to be allocated somewhere. If you make this 
change then you will cause random memory corruption or segmentation 
faults.

> gcc 3.2.2 compiles this properly with only a warning about the possiblity that
> tuple will be uninitialized. The original code looks perfectly legal and proper,
> so why would gcc 3.x fail?

Because the original code is not legal in any C compiler and should not be
accepted by any compiler, not only GCC 3.x (I am using RH GCC 2.96).  The
problem is the IP_CT_TUPLE_BLANK which is an incorrectly constructed
macro.

Regards
Henrik

Re: [PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[Brad Chapman]

Mr. Nordstrom,

--- Henrik Nordstrom <hno@marasystems.com> wrote:
> On Wed, 10 Sep 2003, Brad Chapman wrote:
> 
> > Hi everyone!
> > 
> > gcc fails with the following error when compiling ip_conntrack_core.c on
> 2.4.22-ac2
> > with gcc 3.2.2:
> 
> See my post a few days ago..

OK. I temporarily suspended my subscription a month ago because I was very busy
and no longer had time to read e-mails. I'll check the pipermail archives.

> 
> > +	struct ip_conntrack_tuple *tuple;
> 
> > +	IP_CT_TUPLE_BLANK(tuple);
> 
> Won't work.. the tuple needs to be allocated somewhere. If you make this 
> change then you will cause random memory corruption or segmentation 
> faults.

Hmmmm, OK. Does that mean that the tuple can't be allocated on the stack, but
instead has to be allocated from the heap ([v,k]malloc()?) I had a feeling that
something was amiss, since the original code _appeared_ to be correct, but
did not compile. My patch made it compile, but since I have no apps which use
SO_ORIGINAL_DST, I can't tell if it would cause it to crash.

> 
> > gcc 3.2.2 compiles this properly with only a warning about the possiblity that
> > tuple will be uninitialized. The original code looks perfectly legal and proper,
> > so why would gcc 3.x fail?
> 
> Because the original code is not legal in any C compiler and should not be
> accepted by any compiler, not only GCC 3.x (I am using RH GCC 2.96).  The
> problem is the IP_CT_TUPLE_BLANK which is an incorrectly constructed
> macro.

Then how should it be constructed? It almost sounds like you would need to write
an actual function, and code it and getorigdst() something like this (I'm sure this
is horribly wrong):

static void ip_ct_tuple_blank(struct ip_conntrack_tuple *tuple)
{
         tuple->src.u.all = 0;
         tuple->dst.u.all = 0;
}

static int getorigdst(...)
{
....
         struct ip_conntrack_tuple *tuple;

         tuple = kmalloc(sizeof(ip_conntrack_tuple), GFP_ATOMIC);
         if (!tuple) return -ENOMEM;
         ip_ct_tuple_blank(tuple);
....
         kfree(tuple);
....
}

Would this work, or would it be horribly worse? Where can I get an app which uses
SO_ORIGINAL_DST to test this out?

> 
> Regards
> Henrik
> 

Brad


=====
Brad Chapman

Permanent e-mail: kakadu_croc@yahoo.com

__________________________________
Do you Yahoo!?
Yahoo! SiteBuilder - Free, easy-to-use web site design software
http://sitebuilder.yahoo.com

Re: [PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[dmorris]

>Would this work, or would it be horribly worse? Where can I get an app which uses
>SO_ORIGINAL_DST to test this out?
>
>  
>

Try this:

#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <string.h>
#include <linux/netfilter_ipv4.h>

int main ()
{
    int sock;
    int one = 1;
    struct sockaddr_in addr;
   
    if ((sock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP))<0)
    {perror("socket()");exit(-1);}

    setsockopt(sock, SOL_SOCKET,SO_REUSEADDR,(char *)&one,sizeof(one));

    memset(&addr,0,sizeof(addr));
    addr.sin_port = htons(1234);
    addr.sin_family = AF_INET;
    addr.sin_addr.s_addr = htonl(INADDR_ANY);
    if (bind(sock, (struct sockaddr*)&addr, sizeof(addr))<0)
    {perror("bind()");exit(-1);}
 
    if (listen(sock, 3) < 0)
    {perror("listen()");exit(-1);}
   
    while(1) {
        int addrlen = sizeof(addr);
        int fd = accept(sock,(struct sockaddr*)&addr,&addrlen);
       
        if (fd < 0)
        {perror("accept()");exit(-1);}
      
        printf("accepted.");
       
        if (getsockopt(fd,SOL_IP,SO_ORIGINAL_DST,&addr,&addrlen)<0)
        {perror("getsockopt()");exit(-1);}

        printf("original dest: 
%s:%i\n",inet_ntoa(addr.sin_addr),ntohs(addr.sin_port));

        close(fd);
    }
   
}





then do:
s iptables -t nat -A PREROUTING -p tcp --destination-port 7 -j REDIRECT 
--to-port 1234

then from some machine behind that machine
"telnet 128.2.1.2 7"
Trying 128.2.1.2...
Connected to 128.2.1.2.
Escape character is '^]'.
Connection closed by foreign host.

the machine running the code above should print:
accepted.original dest: 128.2.1.2:7


and from the localmachine
telnet localhost 1234

the code should say:
accepted.original dest: 127.0.0.1:1234

goodluck,

-dirk morris

Re: [PATCH?] Fix getorigdst() in ip_conntrack_core.c on 2.4.22-ac2

[Henrik Nordstrom]

On Wed, 10 Sep 2003, Brad Chapman wrote:

> Hmmmm, OK. Does that mean that the tuple can't be allocated on the stack

It can (and should), but you only allocated a uninitialized pointer not 
the tuple.. The *tuple needs to point at something before you try to 
assign it's members some values..

> static void ip_ct_tuple_blank(struct ip_conntrack_tuple *tuple)
> {
>          tuple->src.u.all = 0;
>          tuple->dst.u.all = 0;
> }

This is the intended semantics of the macro, but due to a missing 
parantesis in the macro it is not how it ends up..

Regards
Henrik