linux-kernel - Re: possible circular locking dependency detected: compound_send_recv+0x189/0x910 [cifs] vm_mmap

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

Message-ID: <20210908142729.GA6873@axis.com>
Date:   Wed, 8 Sep 2021 16:27:29 +0200
From:   Vincent Whitchurch <vincent.whitchurch@...s.com>
To:     Hillf Danton <hdanton@...a.com>, <sfrench@...ba.org>
CC:     Bruno Goncalves <bgoncalv@...hat.com>,
        "linux-cifs@...r.kernel.org" <linux-cifs@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Xiong Zhou <xzhou@...hat.com>,
        Eric Biggers <ebiggers@...gle.com>
Subject: Re: possible circular locking dependency detected:
 compound_send_recv+0x189/0x910 [cifs] vm_mmap_pgoff+0x85/0x160

On Sat, Aug 28, 2021 at 04:02:36AM +0200, Hillf Danton wrote:
> On Fri, 27 Aug 2021 15:27:24 +0200 Vincent Whitchurch wrote:
> > On Fri, Aug 27, 2021 at 10:27:46AM +0200, Hillf Danton wrote:
> > > 
> > > Only if it is too difficult to fix 05946d4b7a73 ("cifs: Fix preauth hash
> > > corruption") within cifs then fix the deadlock by replacing kthread_run()
> > > with queue_work().
> > 
> > Perhaps I'm missing something, but would the lockdep complaint really go
> > away without 05946d4b7a73?  cifs_alloc_hash() is called under the
> > srv_mutex in other places (for example setup_ntlmv2_rsp()), so the
> > 
> > 	&tcp_ses->srv_mutex --> &cpuset_rwsem --> &mm->mmap_lock
> > 
> > chain would still exist, and compound_send_recv() takes srv_mutex before
> > 05946d4b7a73 too, so &mm->mmap_lock -> srv_mutex would exist too.
> 
> Yes you are right. The key is mmap_lock here.
> > 
> > For cifs_alloc_hash() to be able to be called without the srv_mutex I
> > guess it would have to be done when the tcp_ses is allocated.  That
> > however would essentially be a revert of commit 95dc8dd14e2e84cc3ada
> > ("Limit allocation of crypto mechanisms to dialect which requires").
> 
> It is more appreciated to have a fix within cifs.

Yes.  I'm hoping someone else with more insight into the cifs code can
see if there's another way to fix this in cifs or if it's safe to try
and revert 95dc8dd14e2e84cc3ada.  Steve?