[PATCH] ipt_do_table accesss pskb after it has been freed

[PATCH] ipt_do_table accesss pskb after it has been freed

[Dirk Morris]

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
--- linux-2.6.0-test8-orig/net/ipv4/netfilter/ip_tables.c       
2003-10-17 14:42:57.000000000 -0700
+++ linux-2.6.0-test8-netcap2/net/ipv4/netfilter/ip_tables.c    
2003-11-06 18:03:26.000000000 -0800
@@ -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;

Re: [PATCH] ipt_do_table accesss pskb after it has been freed

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 770 bytes --]

On Thu, Nov 06, 2003 at 06:18:15PM -0800, Dirk Morris wrote:
> 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.

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

> -dirk

-- 
- Harald Welte <laforge@netfilter.org>             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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ipt_do_table accesss pskb after it has been freed

[dmorris]

Harald Welte wrote:

>>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.
>>    
>>
>
>no, it's not a bug.  you should return NF_DROP if you want to have the
>skb dropped.
>
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

Re: [PATCH] ipt_do_table accesss pskb after it has been freed

[Henrik Nordstrom]

On Fri, 7 Nov 2003, dmorris wrote:

> 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.

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