From mboxrd@z Thu Jan 1 00:00:00 1970 From: Veaceslav Falico Subject: [PATCH net] af_packet: use spin_lock_bh() for sk_buff_head Date: Wed, 27 Nov 2013 13:57:55 +0100 Message-ID: <1385557075-8896-1-git-send-email-vfalico@redhat.com> Cc: jstancek@redhat.com, Veaceslav Falico , "David S. Miller" , Daniel Borkmann , Willem de Bruijn , Phil Sutter , Eric Dumazet To: netdev@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58087 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752135Ab3K0M5j (ORCPT ); Wed, 27 Nov 2013 07:57:39 -0500 Sender: netdev-owner@vger.kernel.org List-ID: 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" CC: Daniel Borkmann CC: Willem de Bruijn CC: Phil Sutter CC: Eric Dumazet Reported-by: Jan Stancek Tested-by: Jan Stancek Signed-off-by: Veaceslav Falico --- 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: [] packet_set_ring+0x5a0/0x760 [ 1057.951782] {IN-SOFTIRQ-W} state was registered at: [ 1057.956653] [] __lock_acquire+0x59b/0x1a80 [ 1057.962509] [] lock_acquire+0xa2/0x1d0 [ 1057.968001] [] _raw_spin_lock_irqsave+0x56/0x90 [ 1057.974284] [] sock_queue_rcv_skb+0xe5/0x3a0 [ 1057.980306] [] __udpv6_queue_rcv_skb+0x49/0x250 [ 1057.986586] [] udpv6_queue_rcv_skb+0x24b/0x4e0 [ 1057.992773] [] __udp6_lib_rcv+0x147/0x650 [ 1057.998527] [] udpv6_rcv+0x15/0x20 [ 1058.003671] [] ip6_input_finish+0x1e9/0x810 [ 1058.009597] [] ip6_input+0x22/0x60 [ 1058.014754] [] ip6_rcv_finish+0x3f/0x280 [ 1058.020419] [] ipv6_rcv+0x498/0x870 [ 1058.025661] [] __netif_receive_skb_core+0x9f2/0xdb0 [ 1058.032281] [] __netif_receive_skb+0x18/0x60 [ 1058.038294] [] process_backlog+0xbe/0x1a0 [ 1058.044048] [] net_rx_action+0x162/0x370 [ 1058.049713] [] __do_softirq+0x104/0x480 [ 1058.055293] [] do_softirq_own_stack+0x1c/0x30 [ 1058.061393] [] do_softirq+0x7d/0x90 [ 1058.066627] [] local_bh_enable+0xc5/0xd0 [ 1058.072292] [] ip6_finish_output2+0x1fe/0x810 [ 1058.078392] [] ip6_finish_output+0x96/0x220 [ 1058.084317] [] ip6_output+0x4f/0x1e0 [ 1058.089639] [] ip6_local_out+0x29/0x90 [ 1058.095139] [] ip6_push_pending_frames+0x2f0/0x4e0 [ 1058.101680] [] udp_v6_push_pending_frames+0x129/0x3d0 [ 1058.108481] [] udpv6_sendmsg+0x6f6/0xae0 [ 1058.114158] [] inet_sendmsg+0x107/0x220 [ 1058.119738] [] sock_sendmsg+0x8b/0xc0 [ 1058.125145] [] SYSC_sendto+0x124/0x1d0 [ 1058.130636] [] SyS_sendto+0xe/0x10 [ 1058.135792] [] system_call_fastpath+0x16/0x1b [ 1058.141891] irq event stamp: 12417 [ 1058.145287] hardirqs last enabled at (12417): [] __mutex_unlock_slowpath+0xd0/0x1e0 [ 1058.154603] hardirqs last disabled at (12416): [] __mutex_unlock_slowpath+0x46/0x1e0 [ 1058.163916] softirqs last enabled at (12412): [] packet_set_ring+0x2f6/0x760 [ 1058.172616] softirqs last disabled at (12410): [] _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] [ 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: [] 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] [] dump_stack+0x4d/0x66 [ 1058.277072] [] print_usage_bug+0x1ec/0x1fd [ 1058.282815] [] ? print_shortest_lock_dependencies+0x1d0/0x1d0 [ 1058.290196] [] mark_lock+0x21d/0x2a0 [ 1058.295413] [] __lock_acquire+0x5ef/0x1a80 [ 1058.301149] [] ? trace_hardirqs_on+0xd/0x10 [ 1058.306981] [] ? native_sched_clock+0x13/0x80 [ 1058.312983] [] ? sched_clock+0x9/0x10 [ 1058.318287] [] ? native_sched_clock+0x13/0x80 [ 1058.324290] [] ? sched_clock+0x9/0x10 [ 1058.329594] [] ? native_sched_clock+0x13/0x80 [ 1058.335597] [] ? sched_clock+0x9/0x10 [ 1058.340899] [] lock_acquire+0xa2/0x1d0 [ 1058.346287] [] ? packet_set_ring+0x5a0/0x760 [ 1058.352199] [] _raw_spin_lock+0x3e/0x80 [ 1058.357683] [] ? packet_set_ring+0x5a0/0x760 [ 1058.363599] [] packet_set_ring+0x5a0/0x760 [ 1058.369337] [] ? trace_hardirqs_off+0xd/0x10 [ 1058.375254] [] packet_release+0x18d/0x2e0 [ 1058.380910] [] sock_release+0x1f/0x90 [ 1058.386214] [] sock_close+0x12/0x20 [ 1058.391352] [] __fput+0xfd/0x310 [ 1058.396229] [] ____fput+0xe/0x10 [ 1058.401101] [] task_work_run+0xb4/0xe0 [ 1058.406497] [] do_notify_resume+0x9f/0xc0 [ 1058.412153] [] 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