netdev - Re: BUG: held lock freed! - request_sock_queue::rskq

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]

Date:	Thu, 11 Feb 2016 15:54:54 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Ilya Dryomov <idryomov@...il.com>
Cc:	Eric Dumazet <edumazet@...gle.com>,
	netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Ceph Development <ceph-devel@...r.kernel.org>
Subject: Re: BUG: held lock freed! - request_sock_queue::rskq_lock

On Thu, 2016-02-11 at 23:43 +0100, Ilya Dryomov wrote:
> Hi Eric,
> 
> I'm hitting the following splat (attached in full):
> 
> kernel: =========================
> kernel: [ BUG: held lock freed! ]
> kernel: 4.5.0-rc1-ceph-00026-g5e0a311 #1 Not tainted
> kernel: -------------------------
> kernel: swapper/5/0 is freeing memory
> ffff880035c9d200-ffff880035c9dbff, with a lock still held there!
> kernel: (&(&queue->rskq_lock)->rlock){+.-...}, at:
> [<ffffffff816f6a88>] inet_csk_reqsk_queue_add+0x28/0xa0
> kernel: 4 locks held by swapper/5/0:
> kernel: #0:  (rcu_read_lock){......}, at: [<ffffffff8169ef6b>]
> netif_receive_skb_internal+0x4b/0x1f0
> kernel: #1:  (rcu_read_lock){......}, at: [<ffffffff816e977f>]
> ip_local_deliver_finish+0x3f/0x380
> kernel: #2:  (slock-AF_INET){+.-...}, at: [<ffffffff81685ffb>]
> sk_clone_lock+0x19b/0x440
> kernel: #3:  (&(&queue->rskq_lock)->rlock){+.-...}, at:
> [<ffffffff816f6a88>] inet_csk_reqsk_queue_add+0x28/0xa0
> 
> The first couple of occurrences were on 4.4-rc2, a couple of recent ones
> on 4.5-rc1.  I'm suspecting your ebb516af60e1 ("tcp/dccp: fix race at
> listener dismantle phase") and/or a bunch of related commits before it,
> authored in the first half of Oct 2015.
> 
> The spinlock that lockdep complains about is embedded in
> inet_connection_sock, which I think is getting freed in
> inet_child_forget(), called from inet_csk_reqsk_queue_add():
> 
> inet_csk_complete_hashdance
>   inet_csk_reqsk_queue_add
>     spin_lock(&queue->rskq_lock)
>     inet_child_forget
>       reqsk_put
>         reqsk_free
>           sock_put(req->rsk_listener)
>           # rsk_listener is sk that queue is embedded in #
> 
> Your comment in inet_csk_complete_hashdance()
> 
>                 /* Warning: caller must not call reqsk_put(req);
>                  * child stole last reference on it.
>                  */
> 
> suggests that there is only one reference to req at that point in time
> and yet ebb516af60e1 added a reqsk_put() to that path: there was one in
> inet_csk_listen_stop() before, but not in inet_csk_reqsk_queue_add().
> 
> This is from a quick and dirty git blame analysis, so don't shout if
> all of it is wrong...
> 
> Thanks,

Nice catch !

It is a bit late here, I will take a look tomorrow, as it looks
definitely tricky ;)

inet_csk_reqsk_queue_add() should probably return a status.

Even tcp_child_process() looks racy in its access to
parent->sk_data_ready(parent) if parent got freed.

Thanks !



Thanks !