[Openswan dev] [PATCH] Openswan: fix ipcomp skb offset calculations

[David McCullough]

Jivin Florian Westphal lays it down ...
> Paul Wouters <paul at xelerance.com> wrote:
> > I did want to test with your patch, to see if it resolved anything. But
> > it did not compile for me:
> > 
> > make -C /usr/src/kernels/2.6.27.5-117.fc10.x86_64/  BUILDDIR=/vol/git/openswan.ikev2/modobj26 SUBDIRS=/vol/git/openswan.ikev2/modobj26 MODULE_DEF_INCLUDE=/vol/git/openswan.ikev2/packaging/linus/config-all.h MODULE_DEFCONFIG=/vol/git/openswan.ikev2/linux/net/ipsec/defconfig  MODULE_EXTRA_INCLUDE= ARCH=x86_64 modules
> > make[2]: Entering directory `/usr/src/kernels/2.6.27.5-117.fc10.x86_64'
> >   CC [M]  /vol/git/openswan.ikev2/modobj26/ipcomp.o
> > /vol/git/openswan.ikev2/modobj26/ipcomp.c: In function 'skb_copy_ipcomp':
> > /vol/git/openswan.ikev2/modobj26/ipcomp.c:618: error: 'struct sk_buff' has no member named 'h'
> 
> please use skb_transport_header(skb) instead of accessing skb.h, sorry.
> 
> > /vol/git/openswan.ikev2/modobj26/ipcomp.c:626: error: 'struct sk_buff' has no member named 'nh'
> > make[3]: *** [/vol/git/openswan.ikev2/modobj26/ipcomp.o] Error 1
>  
> please use skb_network_header(skb) instead of accessing skb.nh, sorry.
> 
> > What kernel source are you using? Is this a modified or backported kernel?
> 
> 2.6.16.62 (with a lot of backports).
> sk_buff got changed in newer kernels, there are now helpers (skb_network_header() et al.)
> to fetch the network/tranpsort headers (i missed the fact that openswan will define those as
> macros for older kernels, so this is my fault).
> 
> > There is surely something fishy going on with ipcomp, but I'm not sure yet
> > what the problem is.
> 
> I don't know why you don't see a crash, but the code (as it is now) cannot work on 2.6.16.62, as the
> network header of the new skb is passed in as the decompression area (zs.next_out).
> 
> Now, the code sets the network header as follows:
> 
> offset = n->head-skb->head;
> skb_set_network_header(n, offset);
> 
> After macro expansion on 2.6.16, this is:
> 
> n->nh.raw = n->data + (n->head - skb->head);
> 
> which does not work and causes OOOPS with kernel 2.16.62.
> before, it used to be:
> 
> n->nh.raw = skb->nh.raw + (n->head - skb->head);
> 
> IOW, instead of using the original skbs network header pointer, it now uses
> the newly allocated skbs network header pointer as reference to set the newly allocated
> skbs network header pointer 8-).
> 
> with my patch, it will be:
>  headlen = skb->nh.raw - skb->data;
>  skb_set_network_header(n, headlen);
> 
> [I should have written "headlen = skb_network_header(skb) - skb->data" , again, sorry about that]
> 
> which is (after preprocessing):
> n->nh.raw = n->data + (skb->nh.raw - skb->data);
> so, the new skbs network header gets set based on the original skb network header length,
> which is the intention of the original code.
> 
> More information about reproducing kernel crash:
> We see this with kernel 2.6.62 and openswan 2.4.13 running on both endpoints. Plain ping
> usually works because the packet is not compressed (because it is too small), ping -s 1200
> triggers the OOOPS quoted in my original mail.

Ok,  the patch looks ok so far with the above changes (and a few others
on OSW 2.6.19) I have ipcomp working reliably so far.

Thanks for sending in the patch :-)

Paul,  I'll push this one for you pick up.

Cheers,
Davidm

-- 
David McCullough,  david_mccullough at securecomputing.com,   Ph:+61 734352815
Secure Computing - SnapGear  http://www.uCdot.org   http://www.snapgear.com