Re: [aarch64] refcount_t: use-after-free in NFS with 64k pages

[Cristian Marussi <cristian.marussi@arm.com>]

Hi

On 05/02/2019 12:37, Cristian Marussi wrote:
> Hi
> 
> On 05/02/2019 12:14, Benjamin Coddington wrote:
>> On 5 Feb 2019, at 7:10, Cristian Marussi wrote:
>>
>>> Hi Ben
>>>
>>> On 05/02/2019 11:53, Benjamin Coddington wrote:
>>>> Hello Cristian and Punit,
>>>>
>>>> Did you ever get to the bottom of this one?  We just saw this on one 
>>>> run
>>>> of our 4.18.0-era ppc64le, and I'm wondering if we ever found the 
>>>> root
>>>> cause.
>>>
>>> unfortunately I stopped working actively on finding the root cause, 
>>> since I've
>>> found a viable workaround that let us unblock our broken LTP runs.
>>>
>>> Setting wsize=65536 in NFS bootparams completely solves the issue with 
>>> 64k pages
>>> (and does NOT break 4k either :D): this confirmed my hyp that there is 
>>> some sort
>>> of race when accounting refcounts during the lifetime of nfs_page 
>>> structs which
>>> leads to a misscounted refcount...but as I said I never looked back 
>>> into that
>>> again (but never say never...)
>>>
>>> Hope this helps...
>>
>> Hmm, interesting..
>>
>> Will you share your reproducer with me?  That will save me some time.
> 
> Sure.
> 
> My reproducer is the attached nfs_stress.sh script; when invoked with the
> following params:
> 
> ./nfs_stress.sh -w 10 -s 160000 -t 10
> 
> it leads to a crash within 10secs BUT ONLY with 64KB page Kconfig AND ONLY if
> the above wsize workaround is NOT applied. (or the cleanup-code trick mentioned
> in the emails) (the choice of the -s size parameter seemed sensible in determine
> how quick it will die...)
> 
> BUT UNFORTUNATELY this was true ONLY when running on an AEMv8 FastModel (1-cpu
> A53) (whose timings are much different from a real board); I've never been able
> to reproduce reliably on real ARM64 silicon instead. (or on x86)
> So all my debug and triage was made on the model once I was able to quickly
> reproduce the same crash (and in fact the workaround worked then fine also on
> silicon...)
> 
> On real silicon instead the only reproducer was a full LTP run: we had
> consistent failures every night with the same exact refcount stacktrace (but
> every time on a different LTP test as a trigger...being related to NFS activity
> I suppose it's normal); since we applied the wsize workaround we saw no more
> crashes.
> 

I'll try to have a look at my old notes, but afaicr my suspicion that time was
on some NFS code-path in the early life of the nfs_page where a refcount +1 is
set conditional to some other nfs_page flags, so that a race could have happened
between the flags check and the +1 itself (and this could have been influenced
by the fact that a 64kb page took longer to be written out in 4kb wsize chunks...).
One thing which I remember while ftracing is that this thing happened after
after 10secs on a flock of pages...was not only one..you ended up with 3-5 or
more nfs_pages linked between themselves all with a dirty wrong refcount...and
those nfs_pages addresses where perfectly fine till a while before (so the same
nfs_page which I could see going through a clean free with proper refcount, when
re-used later in the test ended up being freed ahead of time due to a miscounted
refcount).

Regards

Cristian
> 
> Thanks
> 
> Regards
> 
> Cristian
> 
>>
>> Ben
>>
>