All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL nf] IPVS
@ 2012-07-11  0:19 Simon Horman
  2012-07-11  0:19 ` [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context Simon Horman
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Simon Horman @ 2012-07-11  0:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer


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(-)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context
  2012-07-11  0:19 [GIT PULL nf] IPVS Simon Horman
@ 2012-07-11  0:19 ` Simon Horman
  2012-07-17 10:08   ` Pablo Neira Ayuso
  2012-07-11  0:19 ` [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn() Simon Horman
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2012-07-11  0:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Lin Ming, Simon Horman

From: Lin Ming <mlin@ss.pku.edu.cn>

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.

[  579.781508] BUG: unable to handle kernel NULL pointer dereference at 0000000000000004
[  579.781669] IP: [<ffffffff817b1ca5>] br_nf_forward_finish+0x58/0x112
[  579.781792] PGD 218f9067 PUD 0
[  579.781865] Oops: 0000 [#1] SMP
[  579.781945] CPU 0
[  579.781983] Modules linked in:
[  579.782047]
[  579.782080]
[  579.782114] Pid: 4644, comm: qemu Tainted: G        W    3.5.0-rc5-00006-g95e69f9 #282 Hewlett-Packard  /30E8
[  579.782300] RIP: 0010:[<ffffffff817b1ca5>]  [<ffffffff817b1ca5>] br_nf_forward_finish+0x58/0x112
[  579.782455] RSP: 0018:ffff88007b003a98  EFLAGS: 00010287
[  579.782541] RAX: 0000000000000008 RBX: ffff8800762ead00 RCX: 000000000001670a
[  579.782653] RDX: 0000000000000000 RSI: 000000000000000a RDI: ffff8800762ead00
[  579.782845] RBP: ffff88007b003ac8 R08: 0000000000016630 R09: ffff88007b003a90
[  579.782957] R10: ffff88007b0038e8 R11: ffff88002da37540 R12: ffff88002da01a02
[  579.783066] R13: ffff88002da01a80 R14: ffff88002d83c000 R15: ffff88002d82a000
[  579.783177] FS:  0000000000000000(0000) GS:ffff88007b000000(0063) knlGS:00000000f62d1b70
[  579.783306] CS:  0010 DS: 002b ES: 002b CR0: 000000008005003b
[  579.783395] CR2: 0000000000000004 CR3: 00000000218fe000 CR4: 00000000000027f0
[  579.783505] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  579.783684] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  579.783795] Process qemu (pid: 4644, threadinfo ffff880021b20000, task ffff880021aba760)
[  579.783919] Stack:
[  579.783959]  ffff88007693cedc ffff8800762ead00 ffff88002da01a02 ffff8800762ead00
[  579.784110]  ffff88002da01a02 ffff88002da01a80 ffff88007b003b18 ffffffff817b26c7
[  579.784260]  ffff880080000000 ffffffff81ef59f0 ffff8800762ead00 ffffffff81ef58b0
[  579.784477] Call Trace:
[  579.784523]  <IRQ>
[  579.784562]
[  579.784603]  [<ffffffff817b26c7>] br_nf_forward_ip+0x275/0x2c8
[  579.784707]  [<ffffffff81704b58>] nf_iterate+0x47/0x7d
[  579.784797]  [<ffffffff817ac32e>] ? br_dev_queue_push_xmit+0xae/0xae
[  579.784906]  [<ffffffff81704bfb>] nf_hook_slow+0x6d/0x102
[  579.784995]  [<ffffffff817ac32e>] ? br_dev_queue_push_xmit+0xae/0xae
[  579.785175]  [<ffffffff8187fa95>] ? _raw_write_unlock_bh+0x19/0x1b
[  579.785179]  [<ffffffff817ac417>] __br_forward+0x97/0xa2
[  579.785179]  [<ffffffff817ad366>] br_handle_frame_finish+0x1a6/0x257
[  579.785179]  [<ffffffff817b2386>] br_nf_pre_routing_finish+0x26d/0x2cb
[  579.785179]  [<ffffffff817b2cf0>] br_nf_pre_routing+0x55d/0x5c1
[  579.785179]  [<ffffffff81704b58>] nf_iterate+0x47/0x7d
[  579.785179]  [<ffffffff817ad1c0>] ? br_handle_local_finish+0x44/0x44
[  579.785179]  [<ffffffff81704bfb>] nf_hook_slow+0x6d/0x102
[  579.785179]  [<ffffffff817ad1c0>] ? br_handle_local_finish+0x44/0x44
[  579.785179]  [<ffffffff81551525>] ? sky2_poll+0xb35/0xb54
[  579.785179]  [<ffffffff817ad62a>] br_handle_frame+0x213/0x229
[  579.785179]  [<ffffffff817ad417>] ? br_handle_frame_finish+0x257/0x257
[  579.785179]  [<ffffffff816e3b47>] __netif_receive_skb+0x2b4/0x3f1
[  579.785179]  [<ffffffff816e69fc>] process_backlog+0x99/0x1e2
[  579.785179]  [<ffffffff816e6800>] net_rx_action+0xdf/0x242
[  579.785179]  [<ffffffff8107e8a8>] __do_softirq+0xc1/0x1e0
[  579.785179]  [<ffffffff8135a5ba>] ? trace_hardirqs_off_thunk+0x3a/0x6c
[  579.785179]  [<ffffffff8188812c>] call_softirq+0x1c/0x30

The steps to reproduce as follow,

1. On Host1, setup brige br0(192.168.1.106)
2. Boot a kvm guest(192.168.1.105) on Host1 and start httpd
3. Start IPVS service on Host1
   ipvsadm -A -t 192.168.1.106:80 -s rr
   ipvsadm -a -t 192.168.1.106:80 -r 192.168.1.105:80 -m
4. Run apache benchmark on Host2(192.168.1.101)
   ab -n 1000 http://192.168.1.106/

ip_vs_reply4
  ip_vs_out
    handle_response
      ip_vs_notrack
        nf_reset()
        {
          skb->nf_bridge = NULL;
        }

Actually, IPVS wants in this case just to replace nfct
with untracked version. So replace the nf_reset(skb) call
in ip_vs_notrack() with a nf_conntrack_put(skb->nfct) call.

Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/net/ip_vs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index d6146b4..95374d1 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1425,7 +1425,7 @@ static inline void ip_vs_notrack(struct sk_buff *skb)
 	struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
 
 	if (!ct || !nf_ct_is_untracked(ct)) {
-		nf_reset(skb);
+		nf_conntrack_put(skb->nfct);
 		skb->nfct = &nf_ct_untracked_get()->ct_general;
 		skb->nfctinfo = IP_CT_NEW;
 		nf_conntrack_get(skb->nfct);
-- 
1.7.10.2.484.gcd07cc5

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()
  2012-07-11  0:19 [GIT PULL nf] IPVS Simon Horman
  2012-07-11  0:19 ` [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context Simon Horman
@ 2012-07-11  0:19 ` Simon Horman
  2012-07-16 21:07   ` Pablo Neira Ayuso
  2012-07-11  0:19 ` [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod Simon Horman
  2012-07-17 10:14 ` [GIT PULL nf] IPVS Pablo Neira Ayuso
  3 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2012-07-11  0:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Xiaotian Feng, Xiaotian Feng, Patrick McHardy, David S. Miller,
	Simon Horman

From: Xiaotian Feng <xtfeng@gmail.com>

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 <dannyfeng@tencent.com>
Cc: Wensong Zhang <wensong@linux-vs.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Patrick McHardy <kaber@trash.net>
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(+)

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index b20b29c..c2bc264 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
 static int
 ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_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);
 	return 0;
 }
 
-- 
1.7.10.2.484.gcd07cc5

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod
  2012-07-11  0:19 [GIT PULL nf] IPVS Simon Horman
  2012-07-11  0:19 ` [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context Simon Horman
  2012-07-11  0:19 ` [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn() Simon Horman
@ 2012-07-11  0:19 ` Simon Horman
  2012-07-17 10:08   ` Pablo Neira Ayuso
  2012-07-17 10:14 ` [GIT PULL nf] IPVS Pablo Neira Ayuso
  3 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2012-07-11  0:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Simon Horman

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(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index d43e3c1..84444dd 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1521,11 +1521,12 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
 {
 	struct net_device *dev = ptr;
 	struct net *net = dev_net(dev);
+	struct netns_ipvs *ipvs = net_ipvs(net);
 	struct ip_vs_service *svc;
 	struct ip_vs_dest *dest;
 	unsigned int idx;
 
-	if (event != NETDEV_UNREGISTER)
+	if (event != NETDEV_UNREGISTER || !ipvs)
 		return NOTIFY_DONE;
 	IP_VS_DBG(3, "%s() dev=%s\n", __func__, dev->name);
 	EnterFunction(2);
@@ -1551,7 +1552,7 @@ static int ip_vs_dst_event(struct notifier_block *this, unsigned long event,
 		}
 	}
 
-	list_for_each_entry(dest, &net_ipvs(net)->dest_trash, n_list) {
+	list_for_each_entry(dest, &ipvs->dest_trash, n_list) {
 		__ip_vs_dev_reset(dest, dev);
 	}
 	mutex_unlock(&__ip_vs_mutex);
-- 
1.7.10.2.484.gcd07cc5


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()
  2012-07-11  0:19 ` [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn() Simon Horman
@ 2012-07-16 21:07   ` Pablo Neira Ayuso
  2012-07-17  2:34     ` Xiaotian Feng
                       ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2012-07-16 21:07 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Xiaotian Feng, Xiaotian Feng, Patrick McHardy, David S. Miller

Hi Simon,

On Wed, Jul 11, 2012 at 09:19:22AM +0900, Simon Horman wrote:
> From: Xiaotian Feng <xtfeng@gmail.com>
> 
> We met a kernel panic in 2.6.32.43 kernel:
[...]
>  net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index b20b29c..c2bc264 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
>  static int
>  ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_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);
>  	return 0;

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()
  2012-07-16 21:07   ` Pablo Neira Ayuso
@ 2012-07-17  2:34     ` Xiaotian Feng
  2012-07-17  5:14     ` Simon Horman
       [not found]     ` <CAJn8CcEChmrFvASChJfj7qK8F-my79fn+-G8ttst02Sts15y6Q@mail.gmail.com>
  2 siblings, 0 replies; 15+ messages in thread
From: Xiaotian Feng @ 2012-07-17  2:34 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Xiaotian Feng, Patrick McHardy, David S. Miller

On Tue, Jul 17, 2012 at 5:07 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Simon,
>
> On Wed, Jul 11, 2012 at 09:19:22AM +0900, Simon Horman wrote:
>> From: Xiaotian Feng <xtfeng@gmail.com>
>>
>> We met a kernel panic in 2.6.32.43 kernel:
> [...]
>>  net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
>> index b20b29c..c2bc264 100644
>> --- a/net/netfilter/ipvs/ip_vs_ftp.c
>> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
>> @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
>>  static int
>>  ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_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);
>>       return 0;
>
> 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.


> Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()
  2012-07-16 21:07   ` Pablo Neira Ayuso
  2012-07-17  2:34     ` Xiaotian Feng
@ 2012-07-17  5:14     ` Simon Horman
       [not found]     ` <CAJn8CcEChmrFvASChJfj7qK8F-my79fn+-G8ttst02Sts15y6Q@mail.gmail.com>
  2 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2012-07-17  5:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Xiaotian Feng, Xiaotian Feng, Patrick McHardy, David S. Miller

On Mon, Jul 16, 2012 at 11:07:57PM +0200, Pablo Neira Ayuso wrote:
> Hi Simon,
> 
> On Wed, Jul 11, 2012 at 09:19:22AM +0900, Simon Horman wrote:
> > From: Xiaotian Feng <xtfeng@gmail.com>
> > 
> > We met a kernel panic in 2.6.32.43 kernel:
> [...]
> >  net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> > index b20b29c..c2bc264 100644
> > --- a/net/netfilter/ipvs/ip_vs_ftp.c
> > +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> > @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
> >  static int
> >  ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_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);
> >  	return 0;
> 
> 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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn()
       [not found]     ` <CAJn8CcEChmrFvASChJfj7qK8F-my79fn+-G8ttst02Sts15y6Q@mail.gmail.com>
@ 2012-07-17  9:46       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2012-07-17  9:46 UTC (permalink / raw)
  To: Xiaotian Feng
  Cc: Simon Horman, lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Xiaotian Feng, Patrick McHardy, David S. Miller

Hi,

On Tue, Jul 17, 2012 at 09:44:01AM +0800, Xiaotian Feng wrote:
> On Tue, Jul 17, 2012 at 5:07 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Simon,
> >
> > On Wed, Jul 11, 2012 at 09:19:22AM +0900, Simon Horman wrote:
> >> From: Xiaotian Feng <xtfeng@gmail.com>
> >>
> >> We met a kernel panic in 2.6.32.43 kernel:
> > [...]
> >>  net/netfilter/ipvs/ip_vs_ftp.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> >> index b20b29c..c2bc264 100644
> >> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> >> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> >> @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv;
> >>  static int
> >>  ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_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);
> >>       return 0;
> >
> > 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.

Please, then fix the patch description and resend the patch to me.

I have to justify why this is pushed forward to David, and using
misleading description for the patch is not the way to go.

Regarding this bitset operation, I think it's way better if you use
bitwise operations for those cp->flags. Getting the spin_lock just to
set the flag is way too much.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context
  2012-07-11  0:19 ` [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context Simon Horman
@ 2012-07-17 10:08   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2012-07-17 10:08 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer,
	Lin Ming

On Wed, Jul 11, 2012 at 09:19:21AM +0900, Simon Horman wrote:
> From: Lin Ming <mlin@ss.pku.edu.cn>
> 
> 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.

Applied, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod
  2012-07-11  0:19 ` [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod Simon Horman
@ 2012-07-17 10:08   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2012-07-17 10:08 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer

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.

Applied, thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL nf] IPVS
  2012-07-11  0:19 [GIT PULL nf] IPVS Simon Horman
                   ` (2 preceding siblings ...)
  2012-07-11  0:19 ` [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod Simon Horman
@ 2012-07-17 10:14 ` Pablo Neira Ayuso
  2012-07-17 13:50   ` Simon Horman
  3 siblings, 1 reply; 15+ messages in thread
From: Pablo Neira Ayuso @ 2012-07-17 10:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer

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.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL nf] IPVS
  2012-07-17 10:14 ` [GIT PULL nf] IPVS Pablo Neira Ayuso
@ 2012-07-17 13:50   ` Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2012-07-17 13:50 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Jesper Dangaard Brouer

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [GIT PULL nf] IPVS
  2013-01-28  1:18 Simon Horman
@ 2013-02-05  2:58 ` Pablo Neira Ayuso
  0 siblings, 0 replies; 15+ messages in thread
From: Pablo Neira Ayuso @ 2013-02-05  2:58 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Hans Schillstrom,
	Jesper Dangaard Brouer, Dan Carpenter

On Mon, Jan 28, 2013 at 10:18:33AM +0900, Simon Horman wrote:
> Hi Pablo,
> 
> please consider the following fix for IPVS patch for 3.8.

Pulled, thanks Simon.

> I believe that the problem was introduced in 3.4 and thus
> this fix is appropriate for 3.4, 3.5, 3.6 and 3.7 -stable.

Will pass this to -stable for 3.4 and 3.7.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [GIT PULL nf] IPVS
@ 2013-01-28  1:18 Simon Horman
  2013-02-05  2:58 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Horman @ 2013-01-28  1:18 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Hans Schillstrom, Hans Schillstrom,
	Jesper Dangaard Brouer, Dan Carpenter

Hi Pablo,

please consider the following fix for IPVS patch for 3.8.

I believe that the problem was introduced in 3.4 and thus
this fix is appropriate for 3.4, 3.5, 3.6 and 3.7 -stable.

----------------------------------------------------------------
The following changes since commit 5b76c4948fe6977bead2359c2054f3e6a2dcf3d0:

  netfilter: x_tables: print correct hook names for ARP (2013-01-13 12:54:12 +0100)

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 b425df4cdd953a400d814b4474c9d3ec04481858:

  ipvs: freeing uninitialized pointer on error (2013-01-28 10:14:37 +0900)

----------------------------------------------------------------
Dan Carpenter (1):
      ipvs: freeing uninitialized pointer on error

 net/netfilter/ipvs/ip_vs_sync.c |    2 ++
 1 file changed, 2 insertions(+)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [GIT PULL nf] IPVS
@ 2012-01-27  1:45 Simon Horman
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2012-01-27  1:45 UTC (permalink / raw)
  To: Patrick McHardy, Pablo Neira Ayuso
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang, Julian Anastasov

Hi Pablo,

please consider pulling
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs.git master
to get the following fix from Julian.

The bug in question has been present since 2.6.37 and accordingly I would
like the fix considered for both 3.3 and stable. I have confirmed that it
applies and builds against your net tree, 3.2.2, 3.1.10, 3.0.18 and 2.6.39.4.

Julian Anastasov (1):
      ipvs: fix matching of fwmark templates during scheduling

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-02-05  2:58 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-11  0:19 [GIT PULL nf] IPVS Simon Horman
2012-07-11  0:19 ` [PATCH 1/3] ipvs: fix oops on NAT reply in br_nf context Simon Horman
2012-07-17 10:08   ` Pablo Neira Ayuso
2012-07-11  0:19 ` [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn() Simon Horman
2012-07-16 21:07   ` Pablo Neira Ayuso
2012-07-17  2:34     ` Xiaotian Feng
2012-07-17  5:14     ` Simon Horman
     [not found]     ` <CAJn8CcEChmrFvASChJfj7qK8F-my79fn+-G8ttst02Sts15y6Q@mail.gmail.com>
2012-07-17  9:46       ` Pablo Neira Ayuso
2012-07-11  0:19 ` [PATCH 3/3] ipvs: fix oops in ip_vs_dst_event on rmmod Simon Horman
2012-07-17 10:08   ` Pablo Neira Ayuso
2012-07-17 10:14 ` [GIT PULL nf] IPVS Pablo Neira Ayuso
2012-07-17 13:50   ` Simon Horman
  -- strict thread matches above, loose matches on Subject: below --
2013-01-28  1:18 Simon Horman
2013-02-05  2:58 ` Pablo Neira Ayuso
2012-01-27  1:45 Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.