[PATCH/RFC] core/netfilter.c: possible use of freed skb

[PATCH/RFC] core/netfilter.c: possible use of freed skb

[Bart De Schuymer]

Hi Dave,

In nf_reinject(), info->okfn(skb) is called when the verdict is NF_ACCEPT.
This okfn deals with the skb and is responsible for freeing the skb.
Unfortunately the current code in nf_reinject() still uses this possibly
freed skb.
The patch below fixes this without introducing any new code.
It should be ok to dev_put the devices earlier, thanks to the
preceding rcu_read_lock.
I also moved the dev_put's of info->{in,out}dev to keep the code together.

cheers,
Bart

--- linux-2.6.0-test1/net/core/netfilter.c.old	2003-07-27 16:22:50.000000000 +0200
+++ linux-2.6.0-test1/net/core/netfilter.c	2003-07-27 16:30:48.000000000 +0200
@@ -557,6 +557,18 @@ void nf_reinject(struct sk_buff *skb, st
 
 	rcu_read_lock();
 
+	/* Release those devices we held, or Alexey will kill me. */
+	if (info->indev) dev_put(info->indev);
+	if (info->outdev) dev_put(info->outdev);
+#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
+	if (skb->nf_bridge) {
+		if (skb->nf_bridge->physindev)
+			dev_put(skb->nf_bridge->physindev);
+		if (skb->nf_bridge->physoutdev)
+			dev_put(skb->nf_bridge->physoutdev);
+	}
+#endif
+
 	/* Drop reference to owner of hook which queued us. */
 	module_put(info->elem->owner);
 
@@ -599,19 +611,6 @@ void nf_reinject(struct sk_buff *skb, st
 	}
 	rcu_read_unlock();
 
-	/* Release those devices we held, or Alexey will kill me. */
-	if (info->indev) dev_put(info->indev);
-	if (info->outdev) dev_put(info->outdev);
-#if defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE)
-	if (skb->nf_bridge) {
-		if (skb->nf_bridge->physindev)
-			dev_put(skb->nf_bridge->physindev);
-		if (skb->nf_bridge->physoutdev)
-			dev_put(skb->nf_bridge->physoutdev);
-	}
-#endif
-
-
 	if (verdict == NF_DROP)
 		kfree_skb(skb);
 

Re: [PATCH/RFC] core/netfilter.c: possible use of freed skb

[David S. Miller]

On Sun, 27 Jul 2003 16:55:15 +0200
Bart De Schuymer <bdschuym@pandora.be> wrote:

> In nf_reinject(), info->okfn(skb) is called when the verdict is NF_ACCEPT.
> This okfn deals with the skb and is responsible for freeing the skb.
> Unfortunately the current code in nf_reinject() still uses this possibly
> freed skb.
> The patch below fixes this without introducing any new code.

This patch looks OK to me, can I get an ACK from the
netfilter folks?

Re: [PATCH/RFC] core/netfilter.c: possible use of freed skb

[Harald Welte]

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

On Sun, Jul 27, 2003 at 03:50:30PM -0700, David S. Miller wrote:

> This patch looks OK to me, can I get an ACK from the
> netfilter folks?

Dave, the patch looks fine to me.

Sorry for the delay, still catching up with everything that was lost
while I was at OLS.

Please apply!

-- 
- 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/RFC] core/netfilter.c: possible use of freed skb

[David S. Miller]

On Wed, 30 Jul 2003 16:06:29 +0200
Harald Welte <laforge@netfilter.org> wrote:

> On Sun, Jul 27, 2003 at 03:50:30PM -0700, David S. Miller wrote:
> 
> > This patch looks OK to me, can I get an ACK from the
> > netfilter folks?
> 
> Dave, the patch looks fine to me.

Applied, thanks guys.