linux-kernel - Re: [PATCH] dma-buf: fix debugfs versus rcu and fence dumping

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

Message-ID: <20181206161950.GB3544@redhat.com>
Date:   Thu, 6 Dec 2018 11:19:51 -0500
From:   Jerome Glisse <jglisse@...hat.com>
To:     "Koenig, Christian" <Christian.Koenig@....com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linaro-mm-sig@...ts.linaro.org" <linaro-mm-sig@...ts.linaro.org>,
        Stéphane Marchesin <marcheu@...omium.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] dma-buf: fix debugfs versus rcu and fence dumping

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@...hat.com:
> >>> From: Jérôme Glisse <jglisse@...hat.com>
> >>>
> >>> 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@...hat.com>
> >>> Cc: Christian König <christian.koenig@....com>
> >>> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> >>> Cc: Sumit Semwal <sumit.semwal@...aro.org>
> >>> Cc: linux-media@...r.kernel.org
> >>> Cc: dri-devel@...ts.freedesktop.org
> >>> Cc: linaro-mm-sig@...ts.linaro.org
> >>> Cc: Stéphane Marchesin <marcheu@...omium.org>
> >>> Cc: stable@...r.kernel.org
> >> 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