linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vincent Whitchurch <vincent.whitchurch@axis.com>
To: Hillf Danton <hdanton@sina.com>, <sfrench@samba.org>
Cc: Bruno Goncalves <bgoncalv@redhat.com>,
	"linux-cifs@vger.kernel.org" <linux-cifs@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Xiong Zhou <xzhou@redhat.com>, Eric Biggers <ebiggers@google.com>
Subject: Re: possible circular locking dependency detected: compound_send_recv+0x189/0x910 [cifs] vm_mmap_pgoff+0x85/0x160
Date: Wed, 8 Sep 2021 16:27:29 +0200	[thread overview]
Message-ID: <20210908142729.GA6873@axis.com> (raw)
In-Reply-To: <20210828020236.2679-1-hdanton@sina.com>

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?

      parent reply	other threads:[~2021-09-08 14:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26 12:05 possible circular locking dependency detected: compound_send_recv+0x189/0x910 [cifs] vm_mmap_pgoff+0x85/0x160 Bruno Goncalves
     [not found] ` <20210827082746.2490-1-hdanton@sina.com>
2021-08-27 13:27   ` Vincent Whitchurch
     [not found]   ` <20210828020236.2679-1-hdanton@sina.com>
2021-09-08 14:27     ` Vincent Whitchurch [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210908142729.GA6873@axis.com \
    --to=vincent.whitchurch@axis.com \
    --cc=bgoncalv@redhat.com \
    --cc=ebiggers@google.com \
    --cc=hdanton@sina.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sfrench@samba.org \
    --cc=xzhou@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).