From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stanislaw Gruszka Subject: Re: [PATCH] net: wireless: iwlegacy: Fix possible data races in il4965_send_rxon_assoc() Date: Fri, 5 Oct 2018 09:54:04 +0200 Message-ID: <20181005075403.GC1931@redhat.com> References: <20181003140745.7650-1-baijiaju1990@gmail.com> <20181004075914.GB20523@redhat.com> <988494cb-c121-697e-b502-ea4e7c601f47@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvalo@codeaurora.org, davem@davemloft.net, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org To: Jia-Ju Bai Return-path: Content-Disposition: inline In-Reply-To: <988494cb-c121-697e-b502-ea4e7c601f47@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Thu, Oct 04, 2018 at 04:52:19PM +0800, Jia-Ju Bai wrote: > On 2018/10/4 15:59, Stanislaw Gruszka wrote: > >On Wed, Oct 03, 2018 at 10:07:45PM +0800, Jia-Ju Bai wrote: > >>These possible races are detected by a runtime testing. > >>To fix these races, the mutex lock is used in il4965_send_rxon_assoc() > >>to protect the data. > >Really ? I'm surprised by that, see below. > > My runtime testing shows that il4965_send_rxon_assoc() and > il4965_configure_filter() are concurrently executed. > But after seeing your reply, I need to carefully check whether my > runtime testing is right, because I think you are right. > In fact, I only monitored the iwl4965 driver, but did not monitor > the iwlegacy driver, so I will do the testing again with monitoring > the lwlegacy driver. > >So I wonder how this patch did not cause the deadlock ? > > Oh, sorry, anyway, my patch will cause double locks... So how those runtime test were performend such you didn't notice this ? > >Anyway what can be done is adding: > > > >lockdep_assert_held(&il->mutex); > > > >il4965_commit_rxon() to check if we hold the mutex. > > I agree. Care to post a patch ? Thanks Stanislaw