Netfilter: kernel panic with REDIRECT target. (2.6.23 and 2.6.23.8)

DISCUSSION David 5 participants 14 messages Nov 18, 2007 – Nov 20, 2007

Subsystems (3)

Networking Network drivers netfilter

lore

Share & tools

lore

David

author

2007-11-18 19:29 UTC #0 lore raw

I'm (very) far from being firewall configuration expert, but I'm seeing a consistent kernel panic when the following rule is triggered.

    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport

5061 --to-ports 5060

(I'm trying to redirect an alternate port to a SIP server)

Am I just being very stupid, or is there something I'm not seeing here?

Thanks
David

Ismail Dönmez

2007-11-18 19:31 UTC #1 lore raw

Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:

Quoted 9 lines

I'm (very) far from being firewall configuration expert, but I'm seeing a consistent kernel panic when the following rule is triggered.
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
(I'm trying to redirect an alternate port to a SIP server)
Am I just being very stupid, or is there something I'm not seeing here?

Also post the kernel panic log.

Signature · 1 line

Faith is believing what you know isn't so -- Mark Twain

David

author

2007-11-18 19:36 UTC #2 lore raw

Quoted 8 lines · Ismail Dönmez

Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:
Quoted 6 lines
I'm (very) far from being firewall configuration expert, but I'm seeing a consistent kernel panic when the following rule is triggered.
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060

Quoted 7 lines

Quoted 4 lines
(I'm trying to redirect an alternate port to a SIP server)
Am I just being very stupid, or is there something I'm not seeing here?
Also post the kernel panic log.

This is a bit difficult since the server is headless (normally). I can try to obtain the panic via a temporary console, but it may have to wait for a day or two.

David

Patrick McHardy

2007-11-18 19:54 UTC #3 lore raw

Quoted 16 lines · David

Quoted 14 lines · Ismail Dönmez
Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:
Quoted 11 lines
I'm (very) far from being firewall configuration expert, but I'm seeing a consistent kernel panic when the following rule is triggered.
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
(I'm trying to redirect an alternate port to a SIP server)
Am I just being very stupid, or is there something I'm not seeing here?

Quoted 7 lines

Quoted 3 lines
Also post the kernel panic log.
This is a bit difficult since the server is headless (normally). I can try to obtain the panic via a temporary console, but it may have to wait for a day or two.

Please try if this patch fixes the problem.

[NETFILTER]: Fix NULL pointer dereference in nf_nat_move_storage()

Reported by Chuck Ebbert as:

	https://bugzilla.redhat.com/show_bug.cgi?id=259501#c14

This routine is called each time hash should be replaced, nf_conn has extension list which contains pointers to connection tracking users (like nat, which is right now the only such user), so when replace takes place it should copy own extensions. Loop above checks for own extension, but tries to move higer-layer one, which can lead to above oops.

Signed-off-by Evgeniy Polyakov <[email protected]>

Signed-off-by David S. Miller <[email protected]>

---
commit 7799652557d966e49512479f4d3b9079bbc01fff
tree c0895084e075bf53697a66816e6bfcbe28d3859d
parent a5a97263a9fd6a94f954d41ae3233ea65a90bd8a
author Evgeniy Polyakov <[email protected]> Thu, 15 Nov 2007 15:52:32 -0800 committer David S. Miller <[email protected]> Thu, 15 Nov 2007 15:52:32 -0800

 net/netfilter/nf_conntrack_extend.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

net/netfilter/nf_conntrack_extend.c +1 -1

index a1a65a1..cf6ba66 100644
@@ -109,7 +109,7 @@ void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
 			rcu_read_lock();
 			t = rcu_dereference(nf_ct_ext_types[i]);
 			if (t && t->move)
-				t->move(ct, ct->ext + ct->ext->offset[id]);
+				t->move(ct, ct->ext + ct->ext->offset[i]);
 			rcu_read_unlock();
 		}
 		kfree(ct->ext);

David

author

2007-11-19 18:52 UTC #4 lore raw

Quoted 8 lines · Patrick McHardy

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
Also post the kernel panic log.
Please try if this patch fixes the problem.

No luck with the patch I'm afraid, panic log attached (of patched kernel).

Thanks
David

Evgeniy Polyakov

2007-11-19 19:27 UTC #5 lore raw

Quoted 11 lines · David

Quoted 8 lines · Patrick McHardy
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
Also post the kernel panic log.
Please try if this patch fixes the problem.
No luck with the patch I'm afraid, panic log attached (of patched kernel).

Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
What is a load on this machine? Is it simple enough to reproduce? I will take closer look tomorrow if this will not help.

Thanks.

net/ipv4/netfilter/nf_nat_core.c +4 -4

index 70e7997..7dc3496 100644
@@ -607,13 +607,13 @@ static void nf_nat_move_storage(struct nf_conn *conntrack, void *old)
 	struct nf_conn_nat *new_nat = nf_ct_ext_find(conntrack, NF_CT_EXT_NAT);
 	struct nf_conn_nat *old_nat = (struct nf_conn_nat *)old;
 	struct nf_conn *ct = old_nat->ct;
-	unsigned int srchash;
+	
+	printk("conntrack: %p, new: %p, old: %p, ct: %p.\n",
+			conntrack, new_nat, old_nat, ct);
 
-	if (!(ct->status & IPS_NAT_DONE_MASK))
+	if (!ct || !(ct->status & IPS_NAT_DONE_MASK))
 		return;
 
-	srchash = hash_by_src(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
-
 	write_lock_bh(&nf_nat_lock);
 	hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
 	new_nat->ct = ct;

Signature · 1 line

	Evgeniy Polyakov

Evgeniy Polyakov

2007-11-19 19:32 UTC #6 lore raw

Quoted 9 lines · Evgeniy Polyakov

Quoted 8 lines · David
Quoted 7 lines · Patrick McHardy
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
Also post the kernel panic log.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Please try if this patch fixes the problem.
No luck with the patch I'm afraid, panic log attached (of patched kernel).
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).

With both patches applied - one Patrick showed and this one.

Signature · 1 line

	Evgeniy Polyakov

David

author

2007-11-19 20:00 UTC #7 lore raw

Quoted 16 lines · Evgeniy Polyakov

Quoted 14 lines · Evgeniy Polyakov
Quoted 12 lines · David
Quoted 10 lines · Patrick McHardy
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
    iptables -t nat -A PREROUTING -j REDIRECT -i eth2 -p udp --dport
5061 --to-ports 5060
Also post the kernel panic log.
Please try if this patch fixes the problem.

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
No luck with the patch I'm afraid, panic log attached (of patched kernel).
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.

Now works, with this in dmesg

conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.

Cheers
David

Evgeniy Polyakov

2007-11-20 11:56 UTC #8 lore raw

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.
Now works, with this in dmesg
conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.

David (Miller :), please apply attached patch, which also needed to fix netfilter connection tracking bug.
When connection tracking entry (nf_conn) is about to copy itself it can have some of its extension users (like nat) as being already freed and thus not required to be copied.
Frankly saying, it can be not the correct fix, but from code observation and test, perfomed by David <[email protected]> it is.

Actually looking at this function I suspect it was copied from nf_nat_setup_info() and thus bug was introduced.

Signed-off-by Evgeniy Polyakov <[email protected]>

net/ipv4/netfilter/nf_nat_core.c +1 -4

index 70e7997..86b465b 100644
@@ -607,13 +607,10 @@ static void nf_nat_move_storage(struct nf_conn *conntrack, void *old)
 	struct nf_conn_nat *new_nat = nf_ct_ext_find(conntrack, NF_CT_EXT_NAT);
 	struct nf_conn_nat *old_nat = (struct nf_conn_nat *)old;
 	struct nf_conn *ct = old_nat->ct;
-	unsigned int srchash;
 
-	if (!(ct->status & IPS_NAT_DONE_MASK))
+	if (!ct || !(ct->status & IPS_NAT_DONE_MASK))
 		return;
 
-	srchash = hash_by_src(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
-
 	write_lock_bh(&nf_nat_lock);
 	hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
 	new_nat->ct = ct;

Signature · 1 line

	Evgeniy Polyakov

David Miller

maintainer

2007-11-20 12:09 UTC #9 lore raw

From: Evgeniy Polyakov <[email protected]>
Date: Tue, 20 Nov 2007 14:55:20 +0300

Quoted 18 lines

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.
Now works, with this in dmesg
conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.
David (Miller :), please apply attached patch, which also needed to fix netfilter connection tracking bug.
When connection tracking entry (nf_conn) is about to copy itself it can have some of its extension users (like nat) as being already freed and thus not required to be copied.
Frankly saying, it can be not the correct fix, but from code observation and test, perfomed by David <[email protected]> it is.

Quoted 4 lines

Actually looking at this function I suspect it was copied from nf_nat_setup_info() and thus bug was introduced.
Signed-off-by Evgeniy Polyakov <[email protected]>

Evgeniy, thanks for figuring this out.

I think it is fair to let Patrick take a quick look at this before it is applied (and Linus is away until next week anyways so there is no rush :-)

I suspect this error might live elsewhere too, so perhaps a good audit should be done for this kind of thing as well so we can kill all such gremlins now.

Thanks again.

Quoted 22 lines

net/ipv4/netfilter/nf_nat_core.c +1 -4

index 70e7997..86b465b 100644
@@ -607,13 +607,10 @@ static void nf_nat_move_storage(struct nf_conn *conntrack, void *old)
 	struct nf_conn_nat *new_nat = nf_ct_ext_find(conntrack, NF_CT_EXT_NAT);
 	struct nf_conn_nat *old_nat = (struct nf_conn_nat *)old;
 	struct nf_conn *ct = old_nat->ct;
-	unsigned int srchash;
 
-	if (!(ct->status & IPS_NAT_DONE_MASK))
+	if (!ct || !(ct->status & IPS_NAT_DONE_MASK))
 		return;
 
-	srchash = hash_by_src(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
-
 	write_lock_bh(&nf_nat_lock);
 	hlist_replace_rcu(&old_nat->bysource, &new_nat->bysource);
 	new_nat->ct = ct;

Signature · 2 lines

-- 
	Evgeniy Polyakov

Patrick McHardy

2007-11-20 12:11 UTC #10 lore raw

In reply to Evgeniy Polyakov

Quoted 9 lines · Evgeniy Polyakov

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.
Now works, with this in dmesg
conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.

Quoted 7 lines

David (Miller :), please apply attached patch, which also needed to fix netfilter connection tracking bug.
When connection tracking entry (nf_conn) is about to copy itself it can have some of its extension users (like nat) as being already freed and thus not required to be copied.
Frankly saying, it can be not the correct fix, but from code observation and test, perfomed by David <[email protected]> it is.

I also don't believe this can be correct, let me look into this first.

Patrick McHardy

2007-11-20 12:24 UTC #11 lore raw

Quoted 18 lines · Patrick McHardy

Quoted 9 lines · Evgeniy Polyakov
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.
Now works, with this in dmesg
conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.
Quoted 7 lines
David (Miller :), please apply attached patch, which also needed to fix netfilter connection tracking bug.
When connection tracking entry (nf_conn) is about to copy itself it can have some of its extension users (like nat) as being already freed and thus not required to be copied.
Frankly saying, it can be not the correct fix, but from code observation and test, perfomed by David <[email protected]> it is.

Quoted 2 lines

I also don't believe this can be correct, let me look into this first.

I now understand whats happening:

- new connection is allocated without helper
- connection is REDIRECTed to localhost
- nf_nat_setup_info adds NAT extension, but doesn't initialize it yet
- nf_conntrack_alter_reply performs a helper lookup based on the new tuple, finds the SIP helper and allocates a helper extension, causing reallocation because of too little space
- nf_nat_move_storage is called with the uninitialized nat extension

So your fix is entirely correct, thanks a lot :)

David Miller

maintainer

2007-11-20 12:27 UTC #12 lore raw

From: Patrick McHardy <[email protected]>
Date: Tue, 20 Nov 2007 13:24:17 +0100

Quoted 11 lines

I now understand whats happening:
- new connection is allocated without helper
- connection is REDIRECTed to localhost
- nf_nat_setup_info adds NAT extension, but doesn't initialize it yet
- nf_conntrack_alter_reply performs a helper lookup based on the new tuple, finds the SIP helper and allocates a helper extension, causing reallocation because of too little space
- nf_nat_move_storage is called with the uninitialized nat extension
So your fix is entirely correct, thanks a lot :)

Great, I've applied Evgeniy's patch.

Evgeniy Polyakov

2007-11-20 13:22 UTC #13 lore raw

In reply to Patrick McHardy

Quoted 33 lines · Patrick McHardy

Quoted 18 lines · Patrick McHardy
Quoted 9 lines · Evgeniy Polyakov
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Ok, let's try it hard way.
Please check attached patch and tell if it helped (it will produce some debug though).
With both patches applied - one Patrick showed and this one.
Now works, with this in dmesg
conntrack: ea94159c, new: ead4d7c4, old: ead4d7d0, ct: 00000000.
Quoted 7 lines
David (Miller :), please apply attached patch, which also needed to fix netfilter connection tracking bug.
When connection tracking entry (nf_conn) is about to copy itself it can have some of its extension users (like nat) as being already freed and thus not required to be copied.
Frankly saying, it can be not the correct fix, but from code observation and test, perfomed by David <[email protected]> it is.
Quoted 2 lines
I also don't believe this can be correct, let me look into this first.
I now understand whats happening:
- new connection is allocated without helper
- connection is REDIRECTed to localhost
- nf_nat_setup_info adds NAT extension, but doesn't initialize it yet
- nf_conntrack_alter_reply performs a helper lookup based on the new tuple, finds the SIP helper and allocates a helper extension, causing reallocation because of too little space
- nf_nat_move_storage is called with the uninitialized nat extension

Quoted 1 line

So your fix is entirely correct, thanks a lot :)

It is always better to check my third eye revelations :) Thanks for checking it.

Signature · 1 line

	Evgeniy Polyakov