netdev - Re: [PATCH net-next] packet: Protect packet sk list with mutex (v2)

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-prev] [day] [month] [year] [list]

Message-Id: <20120822.225945.1565825916747725428.davem@davemloft.net>
Date:	Wed, 22 Aug 2012 22:59:45 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	xemul@...allels.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] packet: Protect packet sk list with mutex (v2)

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Wed, 22 Aug 2012 08:59:17 +0200

> On Tue, 2012-08-21 at 15:06 +0400, Pavel Emelyanov wrote:
>> Change since v1:
>> 
>> * Fixed inuse counters access spotted by Eric
>> 
>> In patch eea68e2f (packet: Report socket mclist info via diag module) I've
>> introduced a "scheduling in atomic" problem in packet diag module -- the
>> socket list is traversed under rcu_read_lock() while performed under it sk
>> mclist access requires rtnl lock (i.e. -- mutex) to be taken.
>> 
>> [152363.820563] BUG: scheduling while atomic: crtools/12517/0x10000002
>> [152363.820573] 4 locks held by crtools/12517:
>> [152363.820581]  #0:  (sock_diag_mutex){+.+.+.}, at: [<ffffffff81a2dcb5>] sock_diag_rcv+0x1f/0x3e
>> [152363.820613]  #1:  (sock_diag_table_mutex){+.+.+.}, at: [<ffffffff81a2de70>] sock_diag_rcv_msg+0xdb/0x11a
>> [152363.820644]  #2:  (nlk->cb_mutex){+.+.+.}, at: [<ffffffff81a67d01>] netlink_dump+0x23/0x1ab
>> [152363.820693]  #3:  (rcu_read_lock){.+.+..}, at: [<ffffffff81b6a049>] packet_diag_dump+0x0/0x1af
>> 
>> Similar thing was then re-introduced by further packet diag patches (fanount 
>> mutex and pgvec mutex for rings) :(
>> 
>> Apart from being terribly sorry for the above, I propose to change the packet
>> sk list protection from spinlock to mutex. This lock currently protects two
>> modifications:
>> 
>> * sklist
>> * prot inuse counters
>> 
>> The sklist modifications can be just reprotected with mutex since they already
>> occur in a sleeping context. The inuse counters modifications are trickier -- the
>> __this_cpu_-s are used inside, thus requiring the caller to handle the potential
>> issues with contexts himself. Since packet sockets' counters are modified in two 
>> places only (packet_create and packet_release) we only need to protect the context 
>> from being preempted. BH disabling is not required in this case.
>> 
>> Signed-off-by: Pavel Emelyanov <xemul@...allels.com>
 ...
> Acked-by: Eric Dumazet <edumazet@...gle.com>

Applied, thanks.
--
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