From: Simon Horman <horms@verge.net.au> Date: 2012-07-11 00:19:20
Hi Pablo,
this pull request consists of three bug fixes for IPVS.
Please consider for inclusion in 3.5 and stable.
The bug fix from Julian, "ipvs: fix oops in ip_vs_dst_event on rmmod"
fixes a regression introduced in 3.4 and thus I believe it is
only relevant to 3.5 and 3.4-stable.
The other two fixes appear to have been present since at least 2.6.37
(there were a lot of changes to IPVS around that time).
----------------------------------------------------------------
The following changes since commit 6bd0405bb4196b44f1acb7a58f11382cdaf6f7f0:
netfilter: nf_ct_ecache: fix crash with multiple containers, one shutting down (2012-07-09 10:53:19 +0200)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
for you to fetch changes up to 51878010232aaac12822e219b94e89de54faa1ef:
ipvs: fix oops in ip_vs_dst_event on rmmod (2012-07-11 09:00:47 +0900)
----------------------------------------------------------------
Julian Anastasov (1):
ipvs: fix oops in ip_vs_dst_event on rmmod
Lin Ming (1):
ipvs: fix oops on NAT reply in br_nf context
Xiaotian Feng (1):
ipvs: add missing lock in ip_vs_ftp_init_conn()
include/net/ip_vs.h | 2 +-
net/netfilter/ipvs/ip_vs_ctl.c | 5 +++--
net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
3 files changed, 6 insertions(+), 3 deletions(-)
From: Simon Horman <horms@verge.net.au> Date: 2012-07-11 00:19:23
From: Julian Anastasov <ja@ssi.bg>
After commit 39f618b4fd95ae243d940ec64c961009c74e3333 (3.4)
"ipvs: reset ipvs pointer in netns" we can oops in
ip_vs_dst_event on rmmod ip_vs because ip_vs_control_cleanup
is called after the ipvs_core_ops subsys is unregistered and
net->ipvs is NULL. Fix it by exiting early from ip_vs_dst_event
if ipvs is NULL. It is safe because all services and dests
for the net are already freed.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Simon Horman <horms@verge.net.au> Date: 2012-07-11 00:19:51
From: Xiaotian Feng <redacted>
We met a kernel panic in 2.6.32.43 kernel:
[2680191.848044] IPVS: ip_vs_conn_hash(): request for already hashed, called from run_timer_softirq+0x175/0x1d0
<snip>
[2680311.849009] general protection fault: 0000 [#1] SMP
[2680311.853001] RIP: 0010:[<ffffffff815f155c>] [<ffffffff815f155c>] ip_vs_conn_expire+0xdc/0x2f0
[2680311.853001] RSP: 0018:ffff880028303e70 EFLAGS: 00010202
[2680311.853001] RAX: dead000000200200 RBX: ffff8801aad00b80 RCX: 0000000000001d90
[2680311.853001] RDX: dead000000100100 RSI: 000000004fd59800 RDI: ffff8801aad00c08
<snip>
[2680311.853001] Call Trace:
[2680311.853001] <IRQ>
[2680311.853001] [<ffffffff815f1480>] ? ip_vs_conn_expire+0x0/0x2f0
[2680311.853001] [<ffffffff8104e2a5>] run_timer_softirq+0x175/0x1d0
[2680311.853001] [<ffffffff81021a48>] ? lapic_next_event+0x18/0x20
[2680311.853001] [<ffffffff81049a13>] __do_softirq+0xb3/0x150
[2680311.853001] [<ffffffff8100cc5c>] call_softirq+0x1c/0x30
[2680311.853001] [<ffffffff8100ea9a>] do_softirq+0x4a/0x80
[2680311.853001] [<ffffffff81049957>] irq_exit+0x77/0x80
[2680311.853001] [<ffffffff81021f2c>] smp_apic_timer_interrupt+0x6c/0xa0
[2680311.853001] [<ffffffff8100c633>] apic_timer_interrupt+0x13/0x20
[2680311.853001] <EOI>
[2680311.853001] [<ffffffff81013b52>] ? mwait_idle+0x52/0x70
[2680311.853001] [<ffffffff8100a7b0>] ? enter_idle+0x20/0x30
[2680311.853001] [<ffffffff8100ac62>] ? cpu_idle+0x52/0x80
[2680311.853001] [<ffffffff816d504d>] ? start_secondary+0x19d/0x280
rax and rdx is LIST_POISON1 and LIST_POISON2, so kernel is list_del() on an already deleted
connection and result the general protect fault.
The "request for already hashed" warning, told us someone might change the connection flags
incorrectly, like described in commit aea9d711, it changes the connection flags, but doesn't
put the connection back to the list. So ip_vs_conn_hash() throw a warning and return.
Later, when ip_vs_conn_expire fire again, ip_vs_conn_unhash() will find the HASHED connection
and list_del() it, then kernel panic happened.
After code review, the only chance that kernel change connection flag without protection is
in ip_vs_ftp_init_conn().
Signed-off-by: Xiaotian Feng <redacted>
Cc: Wensong Zhang <redacted>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
1 file changed, 2 insertions(+)
@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;staticintip_vs_ftp_init_conn(structip_vs_app*app,structip_vs_conn*cp){+spin_lock(&cp->lock);/* We use connection tracking for the command connection */cp->flags|=IP_VS_CONN_F_NFCT;+spin_unlock(&cp->lock);return0;}
@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;staticintip_vs_ftp_init_conn(structip_vs_app*app,structip_vs_conn*cp){+spin_lock(&cp->lock);/* We use connection tracking for the command connection */cp->flags|=IP_VS_CONN_F_NFCT;+spin_unlock(&cp->lock);return0;
The conntrack support for FTP IPVS helper seems to be there since
2.6.37.
However, the patch description mentions 2.6.32.43.
Something doesn't match here, could you clarify this?
Thanks.
@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;staticintip_vs_ftp_init_conn(structip_vs_app*app,structip_vs_conn*cp){+spin_lock(&cp->lock);/* We use connection tracking for the command connection */cp->flags|=IP_VS_CONN_F_NFCT;+spin_unlock(&cp->lock);return0;
The conntrack support for FTP IPVS helper seems to be there since
2.6.37.
However, the patch description mentions 2.6.32.43.
Something doesn't match here, could you clarify this?
Sorry for the misleading description in the patch. We found the panic
in 2.6.32.43 is caused by changing cp->flags without protection. In
2.6.32.43, ip_vs_process_message changes cp->flags without protection
while update active/inactive flags for the connection.
After code inspiration, we found in 3.x kernel, it is accidentally
fixed by commit f73181c. But with ip_vs_app changes,
ip_vs_ftp_init_conn() will have chance to change cp->flags without
protection. So it is a potential bug in 3.x kernel.
@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;staticintip_vs_ftp_init_conn(structip_vs_app*app,structip_vs_conn*cp){+spin_lock(&cp->lock);/* We use connection tracking for the command connection */cp->flags|=IP_VS_CONN_F_NFCT;+spin_unlock(&cp->lock);return0;
The conntrack support for FTP IPVS helper seems to be there since
2.6.37.
However, the patch description mentions 2.6.32.43.
Something doesn't match here, could you clarify this?
My understanding is that the problem was observed with 2.6.32
but that it has been around for much longer.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-07-17 10:08:33
On Wed, Jul 11, 2012 at 09:19:21AM +0900, Simon Horman wrote:
From: Lin Ming <redacted>
IPVS should not reset skb->nf_bridge in FORWARD hook
by calling nf_reset for NAT replies. It triggers oops in
br_nf_forward_finish.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-07-17 10:08:53
On Wed, Jul 11, 2012 at 09:19:23AM +0900, Simon Horman wrote:
From: Julian Anastasov <ja@ssi.bg>
After commit 39f618b4fd95ae243d940ec64c961009c74e3333 (3.4)
"ipvs: reset ipvs pointer in netns" we can oops in
ip_vs_dst_event on rmmod ip_vs because ip_vs_control_cleanup
is called after the ipvs_core_ops subsys is unregistered and
net->ipvs is NULL. Fix it by exiting early from ip_vs_dst_event
if ipvs is NULL. It is safe because all services and dests
for the net are already freed.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-07-17 10:14:06
On Wed, Jul 11, 2012 at 09:19:20AM +0900, Simon Horman wrote:
Hi Pablo,
this pull request consists of three bug fixes for IPVS.
Please consider for inclusion in 3.5 and stable.
The bug fix from Julian, "ipvs: fix oops in ip_vs_dst_event on rmmod"
fixes a regression introduced in 3.4 and thus I believe it is
only relevant to 3.5 and 3.4-stable.
The other two fixes appear to have been present since at least 2.6.37
(there were a lot of changes to IPVS around that time).
I have passed the two of these patches to David. The one for the FTP
needs a consistent description.
It's fairly late in the development cycle (-rc7), but these are small.
Let's see if David is still in time to accept them. Otherwise, they go
to net-next and we will ask for -stable submission.
From: Simon Horman <horms@verge.net.au> Date: 2012-07-17 13:50:18
On Tue, Jul 17, 2012 at 12:14:06PM +0200, Pablo Neira Ayuso wrote:
On Wed, Jul 11, 2012 at 09:19:20AM +0900, Simon Horman wrote:
quoted
Hi Pablo,
this pull request consists of three bug fixes for IPVS.
Please consider for inclusion in 3.5 and stable.
The bug fix from Julian, "ipvs: fix oops in ip_vs_dst_event on rmmod"
fixes a regression introduced in 3.4 and thus I believe it is
only relevant to 3.5 and 3.4-stable.
The other two fixes appear to have been present since at least 2.6.37
(there were a lot of changes to IPVS around that time).
I have passed the two of these patches to David. The one for the FTP
needs a consistent description.
It's fairly late in the development cycle (-rc7), but these are small.
Let's see if David is still in time to accept them. Otherwise, they go
to net-next and we will ask for -stable submission.
Thanks, it seems that David was in an accepting mood.