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

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

[David]

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

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

[Ismail Dönmez]

Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:
> 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.


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

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

[David]

Ismail Dönmez wrote:
> Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:
>   
>> 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.
>   
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

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

[Patrick McHardy]

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

David wrote:
> Ismail Dönmez wrote:
>   
>> Sunday 18 November 2007 Tarihinde 21:00:12 yazmıştı:
>>   
>>     
>>> 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.
>>   
>>     
> 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.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1545 bytes --]

[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 <johnpol@2ka.mipt.ru>
Signed-off-by: David S. Miller <davem@davemloft.net>

---
commit 7799652557d966e49512479f4d3b9079bbc01fff
tree c0895084e075bf53697a66816e6bfcbe28d3859d
parent a5a97263a9fd6a94f954d41ae3233ea65a90bd8a
author Evgeniy Polyakov <johnpol@2ka.mipt.ru> Thu, 15 Nov 2007 15:52:32 -0800
committer David S. Miller <davem@davemloft.net> Thu, 15 Nov 2007 15:52:32 -0800

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

diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index a1a65a1..cf6ba66 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -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);

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

[David]

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

Patrick McHardy wrote:
>>>>     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

[-- Attachment #2: 19112007003.jpg --]
[-- Type: image/jpeg, Size: 80246 bytes --]

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

[Evgeniy Polyakov]

On Mon, Nov 19, 2007 at 06:51:38PM +0000, David (david@unsolicited.net) wrote:
> Patrick McHardy wrote:
> >>>>     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.

diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 70e7997..7dc3496 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -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;

-- 
	Evgeniy Polyakov

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

[Evgeniy Polyakov]

On Mon, Nov 19, 2007 at 10:24:23PM +0300, Evgeniy Polyakov (johnpol@2ka.mipt.ru) wrote:
> On Mon, Nov 19, 2007 at 06:51:38PM +0000, David (david@unsolicited.net) wrote:
> > Patrick McHardy wrote:
> > >>>>     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).

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

-- 
	Evgeniy Polyakov

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

[David]

Evgeniy Polyakov wrote:
> On Mon, Nov 19, 2007 at 10:24:23PM +0300, Evgeniy Polyakov (johnpol@2ka.mipt.ru) wrote:
>   
>> On Mon, Nov 19, 2007 at 06:51:38PM +0000, David (david@unsolicited.net) wrote:
>>     
>>> Patrick McHardy wrote:
>>>       
>>>>>>>     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).
>>     
>
> 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

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

[Evgeniy Polyakov]

> >> 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 <david@unsolicited.net> 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 <johnpol@2ka.mipt.ru>

diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 70e7997..86b465b 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -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;

-- 
	Evgeniy Polyakov

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

[David Miller]

From: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Date: Tue, 20 Nov 2007 14:55:20 +0300

> > >> 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 <david@unsolicited.net> 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 <johnpol@2ka.mipt.ru>

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.

> diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
> index 70e7997..86b465b 100644
> --- a/net/ipv4/netfilter/nf_nat_core.c
> +++ b/net/ipv4/netfilter/nf_nat_core.c
> @@ -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;
> 
> -- 
> 	Evgeniy Polyakov

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

[Patrick McHardy]

Evgeniy Polyakov wrote:
>>>> 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 <david@unsolicited.net> it is.

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

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

[Patrick McHardy]

Patrick McHardy wrote:
> Evgeniy Polyakov wrote:
>>>>> 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 <david@unsolicited.net> it is.
> 
> 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 :)

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

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Tue, 20 Nov 2007 13:24:17 +0100

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

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

[Evgeniy Polyakov]

On Tue, Nov 20, 2007 at 01:24:17PM +0100, Patrick McHardy (kaber@trash.net) wrote:
> Patrick McHardy wrote:
> >Evgeniy Polyakov wrote:
> >>>>>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 <david@unsolicited.net> it is.
> >
> >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 :)

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

-- 
	Evgeniy Polyakov