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

[Florian Westphal]

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.

Florian