ipt_do_table accesss pskb after it has been freed

I had a target in the mangle/PREROUTING that was freeing the skb and returning NF_STOLEN. ipt_do_table would then cause a kernel panic as it accesses the skb after its freed.

Is this a bug? If so, here's the patch.

-dirk

-->

diff -urN --ignore-all-space linux-2.6.0-test8-orig/net/ipv4/netfilter/ip_tables.c linux-2.6.0-test8-netcap2/net/ipv4/netfilter/ip_tables.c
@@ -375,8 +375,10 @@
                                      = 0x57acc001;

#endif
                              /* Target might have changed stuff. */
+              if (verdict != NF_STOLEN) {
                              ip = (*pskb)->nh.iph;
                              datalen = (*pskb)->len - ip->ihl * 4;

+ }

                              if (verdict == IPT_CONTINUE)
                                      e = (void *)e + e->next_offset;

no, it's not a bug. you should return NF_DROP if you want to have the skb dropped.

- Harald Welte <[email protected]>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

What am I supposed to do if there is no skb to be dropped. My hook calls some other functions in the kernel which call skb_free if all is not well.
That leaves me with two choices, either make a copy of the skb, which i cant do for performance reasons, or apply this patch.

Do I have other options?
Does the netfilter code assume that skb_free cannot be called from within hooks? and do you see any unintended consequences of this hack?

-dirk

You don't need to copy the skb, but you need to protect it from beeing freed if what you are calling is destructive and may call kfree_skb():

i.e. something like this:

   struct skbuff *ref = skb_get(skb);

/* call possibly destructive function */

   if (was_not_destructive) {
       skb_free(ref);
       return NF_ACCEPT;
   } else {
       /* our reference was eaten by the evil destructive call above */B
       return NF_DROP;
   }

Regards
Henrik