| From: Julian Anastasov <ja@ssi.bg> |
| Date: Thu, 3 May 2018 22:02:18 +0300 |
| Subject: ipvs: fix stats update from local clients |
| |
| commit d5e032fc5697b6c0d6b4958bcacb981a08f8174e upstream. |
| |
| Local clients are not properly synchronized on 32-bit CPUs when |
| updating stats (3.10+). Now it is possible estimation_timer (timer), |
| a stats reader, to interrupt the local client in the middle of |
| write_seqcount_{begin,end} sequence leading to loop (DEADLOCK). |
| The same interrupt can happen from received packet (SoftIRQ) |
| which updates the same per-CPU stats. |
| |
| Fix it by disabling BH while updating stats. |
| |
| Found with debug: |
| |
| WARNING: inconsistent lock state |
| 4.17.0-rc2-00105-g35cb6d7-dirty #2 Not tainted |
| -------------------------------- |
| inconsistent {IN-SOFTIRQ-R} -> {SOFTIRQ-ON-W} usage. |
| ftp/2545 [HC0[0]:SC0[0]:HE1:SE1] takes: |
| 86845479 (&syncp->seq#6){+.+-}, at: ip_vs_schedule+0x1c5/0x59e [ip_vs] |
| {IN-SOFTIRQ-R} state was registered at: |
| lock_acquire+0x44/0x5b |
| estimation_timer+0x1b3/0x341 [ip_vs] |
| call_timer_fn+0x54/0xcd |
| run_timer_softirq+0x10c/0x12b |
| __do_softirq+0xc1/0x1a9 |
| do_softirq_own_stack+0x1d/0x23 |
| irq_exit+0x4a/0x64 |
| smp_apic_timer_interrupt+0x63/0x71 |
| apic_timer_interrupt+0x3a/0x40 |
| default_idle+0xa/0xc |
| arch_cpu_idle+0x9/0xb |
| default_idle_call+0x21/0x23 |
| do_idle+0xa0/0x167 |
| cpu_startup_entry+0x19/0x1b |
| start_secondary+0x133/0x182 |
| startup_32_smp+0x164/0x168 |
| irq event stamp: 42213 |
| |
| other info that might help us debug this: |
| Possible unsafe locking scenario: |
| |
| CPU0 |
| ---- |
| lock(&syncp->seq#6); |
| <Interrupt> |
| lock(&syncp->seq#6); |
| |
| *** DEADLOCK *** |
| |
| Fixes: ac69269a45e8 ("ipvs: do not disable bh for long time") |
| Signed-off-by: Julian Anastasov <ja@ssi.bg> |
| Acked-by: Simon Horman <horms@verge.net.au> |
| Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> |
| [bwh: Backported to 3.16: |
| - Drop change in ip_vs_conn_stats(), which doesn't use a seqlock |
| - Adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| --- a/net/netfilter/ipvs/ip_vs_core.c |
| +++ b/net/netfilter/ipvs/ip_vs_core.c |
| @@ -118,6 +118,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, st |
| struct ip_vs_cpu_stats *s; |
| struct ip_vs_service *svc; |
| |
| + local_bh_disable(); |
| + |
| s = this_cpu_ptr(dest->stats.cpustats); |
| s->ustats.inpkts++; |
| u64_stats_update_begin(&s->syncp); |
| @@ -138,6 +140,8 @@ ip_vs_in_stats(struct ip_vs_conn *cp, st |
| u64_stats_update_begin(&s->syncp); |
| s->ustats.inbytes += skb->len; |
| u64_stats_update_end(&s->syncp); |
| + |
| + local_bh_enable(); |
| } |
| } |
| |
| @@ -152,6 +156,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, s |
| struct ip_vs_cpu_stats *s; |
| struct ip_vs_service *svc; |
| |
| + local_bh_disable(); |
| + |
| s = this_cpu_ptr(dest->stats.cpustats); |
| s->ustats.outpkts++; |
| u64_stats_update_begin(&s->syncp); |
| @@ -172,6 +178,8 @@ ip_vs_out_stats(struct ip_vs_conn *cp, s |
| u64_stats_update_begin(&s->syncp); |
| s->ustats.outbytes += skb->len; |
| u64_stats_update_end(&s->syncp); |
| + |
| + local_bh_enable(); |
| } |
| } |
| |
