On Thu, Dec 06, 2018 at 04:08:12PM +0000, Koenig, Christian wrote: > Am 06.12.18 um 16:21 schrieb Jerome Glisse: > > On Thu, Dec 06, 2018 at 08:09:28AM +0000, Koenig, Christian wrote: > >> Am 06.12.18 um 02:41 schrieb jglisse@xxxxxxxxxx: > >>> From: Jérôme Glisse <jglisse@xxxxxxxxxx> > >>> > >>> The debugfs take reference on fence without dropping them. Also the > >>> rcu section are not well balance. Fix all that ... > >>> > >>> Signed-off-by: Jérôme Glisse <jglisse@xxxxxxxxxx> > >>> Cc: Christian König <christian.koenig@xxxxxxx> > >>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > >>> Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx> > >>> Cc: linux-media@xxxxxxxxxxxxxxx > >>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > >>> Cc: linaro-mm-sig@xxxxxxxxxxxxxxxx > >>> Cc: Stéphane Marchesin <marcheu@xxxxxxxxxxxx> > >>> Cc: stable@xxxxxxxxxxxxxxx > >> Well NAK, you are now taking the RCU lock twice and dropping the RCU and > >> still accessing fobj has a huge potential for accessing freed up memory. > >> > >> The only correct thing I can see here is to grab a reference to the > >> fence before printing any info on it, > >> Christian. > > Hu ? That is exactly what i am doing, take reference under rcu, > > rcu_unlock print the fence info, drop the fence reference, rcu > > lock rinse and repeat ... > > > > Note that the fobj in _existing_ code is access outside the rcu > > end that there is an rcu imbalance in that code ie a lonlely > > rcu_unlock after the for loop. > > > > So that the existing code is broken. > > No, the existing code is perfectly fine. > > Please note the break in the loop before the rcu_unlock(); > > if (!read_seqcount_retry(&robj->seq, seq)) > > break; <- HERE! > > rcu_read_unlock(); > > } > > So your patch breaks that and take the RCU read lock twice. Ok missed that, i wonder if the refcount in balance explains the crash that was reported to me ... i sent a patch just for that. Thank you for reviewing and pointing out the code i was oblivious too :) Cheers, Jérôme
