lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 27 Nov 2013 13:57:55 +0100
From:	Veaceslav Falico <vfalico@...hat.com>
To:	netdev@...r.kernel.org
Cc:	jstancek@...hat.com, Veaceslav Falico <vfalico@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Daniel Borkmann <dborkman@...hat.com>,
	Willem de Bruijn <willemb@...gle.com>,
	Phil Sutter <phil@....cc>, Eric Dumazet <edumazet@...gle.com>
Subject: [PATCH net] af_packet: use spin_lock_bh() for sk_buff_head

While running a debug kernel, a warning of inconsistent lock state showed
up for sk->sk_receive_queue->lock between packet_set_ring() (actually,
static prb_shutdown_retire_blk_timer(), which does spin_lock()) and
sock_queue_rcv_skb(), which does spin_lock_irqsave().

Fix this by converting the spin_lock() used in
prb_shutdown_retire_blk_timer() to spin_lock_bh(), as it is used in
packet_set_ring().

CC: "David S. Miller" <davem@...emloft.net>
CC: Daniel Borkmann <dborkman@...hat.com>
CC: Willem de Bruijn <willemb@...gle.com>
CC: Phil Sutter <phil@....cc>
CC: Eric Dumazet <edumazet@...gle.com>
Reported-by: Jan Stancek <jstancek@...hat.com>
Tested-by: Jan Stancek <jstancek@...hat.com>
Signed-off-by: Veaceslav Falico <vfalico@...hat.com>
---

Notes:
    I'm not sure if it's the proper fix - as it might hide bigger problems,
    and I'm not really comfortable with the current mixing spin_lock_irqsave()
    and spin_lock_bh().
    
    The complete warning is:
    
    [ 1057.914735] =================================
    [ 1057.919091] [ INFO: inconsistent lock state ]
    [ 1057.923443] 3.13.0-rc1+ #1 Not tainted
    [ 1057.927186] ---------------------------------
    [ 1057.931536] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
    [ 1057.937541] dumpcap/9377 [HC0[0]:SC0[0]:HE1:SE1] takes:
    [ 1057.942764]  (&(&list->lock)->rlock){+.?...}, at: [<ffffffff81673ee0>] packet_set_ring+0x5a0/0x760
    [ 1057.951782] {IN-SOFTIRQ-W} state was registered at:
    [ 1057.956653]   [<ffffffff810d4c7b>] __lock_acquire+0x59b/0x1a80
    [ 1057.962509]   [<ffffffff810d6952>] lock_acquire+0xa2/0x1d0
    [ 1057.968001]   [<ffffffff816a3916>] _raw_spin_lock_irqsave+0x56/0x90
    [ 1057.974284]   [<ffffffff81545d55>] sock_queue_rcv_skb+0xe5/0x3a0
    [ 1057.980306]   [<ffffffff8164a749>] __udpv6_queue_rcv_skb+0x49/0x250
    [ 1057.986586]   [<ffffffff8164c0cb>] udpv6_queue_rcv_skb+0x24b/0x4e0
    [ 1057.992773]   [<ffffffff8164c7e7>] __udp6_lib_rcv+0x147/0x650
    [ 1057.998527]   [<ffffffff8164cd05>] udpv6_rcv+0x15/0x20
    [ 1058.003671]   [<ffffffff8162cc99>] ip6_input_finish+0x1e9/0x810
    [ 1058.009597]   [<ffffffff8162db52>] ip6_input+0x22/0x60
    [ 1058.014754]   [<ffffffff8162c86f>] ip6_rcv_finish+0x3f/0x280
    [ 1058.020419]   [<ffffffff8162d758>] ipv6_rcv+0x498/0x870
    [ 1058.025661]   [<ffffffff8155c312>] __netif_receive_skb_core+0x9f2/0xdb0
    [ 1058.032281]   [<ffffffff8155c6e8>] __netif_receive_skb+0x18/0x60
    [ 1058.038294]   [<ffffffff8155ec8e>] process_backlog+0xbe/0x1a0
    [ 1058.044048]   [<ffffffff8155e9c2>] net_rx_action+0x162/0x370
    [ 1058.049713]   [<ffffffff8107ae84>] __do_softirq+0x104/0x480
    [ 1058.055293]   [<ffffffff816aedbc>] do_softirq_own_stack+0x1c/0x30
    [ 1058.061393]   [<ffffffff8107b2ed>] do_softirq+0x7d/0x90
    [ 1058.066627]   [<ffffffff8107b495>] local_bh_enable+0xc5/0xd0
    [ 1058.072292]   [<ffffffff816287ee>] ip6_finish_output2+0x1fe/0x810
    [ 1058.078392]   [<ffffffff8162c426>] ip6_finish_output+0x96/0x220
    [ 1058.084317]   [<ffffffff8162c5ff>] ip6_output+0x4f/0x1e0
    [ 1058.089639]   [<ffffffff8166fc19>] ip6_local_out+0x29/0x90
    [ 1058.095139]   [<ffffffff8162a060>] ip6_push_pending_frames+0x2f0/0x4e0
    [ 1058.101680]   [<ffffffff81649719>] udp_v6_push_pending_frames+0x129/0x3d0
    [ 1058.108481]   [<ffffffff8164a296>] udpv6_sendmsg+0x6f6/0xae0
    [ 1058.114158]   [<ffffffff815e5aa7>] inet_sendmsg+0x107/0x220
    [ 1058.119738]   [<ffffffff8154005b>] sock_sendmsg+0x8b/0xc0
    [ 1058.125145]   [<ffffffff81540204>] SYSC_sendto+0x124/0x1d0
    [ 1058.130636]   [<ffffffff8154149e>] SyS_sendto+0xe/0x10
    [ 1058.135792]   [<ffffffff816ad3e9>] system_call_fastpath+0x16/0x1b
    [ 1058.141891] irq event stamp: 12417
    [ 1058.145287] hardirqs last  enabled at (12417): [<ffffffff816a16e0>] __mutex_unlock_slowpath+0xd0/0x1e0
    [ 1058.154603] hardirqs last disabled at (12416): [<ffffffff816a1656>] __mutex_unlock_slowpath+0x46/0x1e0
    [ 1058.163916] softirqs last  enabled at (12412): [<ffffffff81673c36>] packet_set_ring+0x2f6/0x760
    [ 1058.172616] softirqs last disabled at (12410): [<ffffffff816a2e28>] _raw_spin_lock_bh+0x18/0x80
    [ 1058.181314]
    [ 1058.181314] other info that might help us debug this:
    [ 1058.187829]  Possible unsafe locking scenario:
    [ 1058.187829]
    [ 1058.193739]        CPU0
    [ 1058.196188]        ----
    [ 1058.198634]   lock(&(&list->lock)->rlock);
    [ 1058.202756]   <Interrupt>
    [ 1058.205373]     lock(&(&list->lock)->rlock);
    [ 1058.209670]
    [ 1058.209670]  *** DEADLOCK ***
    [ 1058.209670]
    [ 1058.215580] 1 lock held by dumpcap/9377:
    [ 1058.219495]  #0:  (sk_lock-AF_PACKET){+.+.+.}, at: [<ffffffff81673b88>] packet_set_ring+0x248/0x760
    [ 1058.228612]
    [ 1058.228612] stack backtrace:
    [ 1058.232960] CPU: 1 PID: 9377 Comm: dumpcap Not tainted 3.13.0-rc1+ #1
    [ 1058.239390] Hardware name: Dell Inc. PowerEdge R210 II/09T7VV, BIOS 2.0.4 02/29/2012
    [ 1058.247117]  ffffffff82688fe0 ffff880132fedb78 ffffffff81699e80 ffff880138348000
    [ 1058.254569]  ffff880132fedbc8 ffffffff81693c09 0000000000000000 ffff880100000001
    [ 1058.262030]  ffffffff00000001 0000000000000004 ffff880138348000 ffffffff810d3360
    [ 1058.269488] Call Trace:
    [ 1058.271933]  [<ffffffff81699e80>] dump_stack+0x4d/0x66
    [ 1058.277072]  [<ffffffff81693c09>] print_usage_bug+0x1ec/0x1fd
    [ 1058.282815]  [<ffffffff810d3360>] ? print_shortest_lock_dependencies+0x1d0/0x1d0
    [ 1058.290196]  [<ffffffff810d3f5d>] mark_lock+0x21d/0x2a0
    [ 1058.295413]  [<ffffffff810d4ccf>] __lock_acquire+0x5ef/0x1a80
    [ 1058.301149]  [<ffffffff810d42fd>] ? trace_hardirqs_on+0xd/0x10
    [ 1058.306981]  [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
    [ 1058.312983]  [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
    [ 1058.318287]  [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
    [ 1058.324290]  [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
    [ 1058.329594]  [<ffffffff81022f83>] ? native_sched_clock+0x13/0x80
    [ 1058.335597]  [<ffffffff81022ff9>] ? sched_clock+0x9/0x10
    [ 1058.340899]  [<ffffffff810d6952>] lock_acquire+0xa2/0x1d0
    [ 1058.346287]  [<ffffffff81673ee0>] ? packet_set_ring+0x5a0/0x760
    [ 1058.352199]  [<ffffffff816a2dce>] _raw_spin_lock+0x3e/0x80
    [ 1058.357683]  [<ffffffff81673ee0>] ? packet_set_ring+0x5a0/0x760
    [ 1058.363599]  [<ffffffff81673ee0>] packet_set_ring+0x5a0/0x760
    [ 1058.369337]  [<ffffffff810d07dd>] ? trace_hardirqs_off+0xd/0x10
    [ 1058.375254]  [<ffffffff8167422d>] packet_release+0x18d/0x2e0
    [ 1058.380910]  [<ffffffff8153f55f>] sock_release+0x1f/0x90
    [ 1058.386214]  [<ffffffff8153f5e2>] sock_close+0x12/0x20
    [ 1058.391352]  [<ffffffff8120702d>] __fput+0xfd/0x310
    [ 1058.396229]  [<ffffffff8120728e>] ____fput+0xe/0x10
    [ 1058.401101]  [<ffffffff8109cdf4>] task_work_run+0xb4/0xe0
    [ 1058.406497]  [<ffffffff8101aaff>] do_notify_resume+0x9f/0xc0
    [ 1058.412153]  [<ffffffff816ad722>] int_signal+0x12/0x17

 net/packet/af_packet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index ac27c86..ba2548b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -439,9 +439,9 @@ static void prb_shutdown_retire_blk_timer(struct packet_sock *po,
 
 	pkc = tx_ring ? &po->tx_ring.prb_bdqc : &po->rx_ring.prb_bdqc;
 
-	spin_lock(&rb_queue->lock);
+	spin_lock_bh(&rb_queue->lock);
 	pkc->delete_blk_timer = 1;
-	spin_unlock(&rb_queue->lock);
+	spin_unlock_bh(&rb_queue->lock);
 
 	prb_del_retire_blk_timer(pkc);
 }
-- 
1.8.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ