Re: Memory modified after free in "MAP ENTRY" zone (vm_map_entry_t->read_ahead)

On 15/02/2016 00:27, Alan Cox wrote:
> 
> On Sun, Feb 14, 2016 at 8:09 AM, Andriy Gapon <avg_at_freebsd.org
> <mailto:avg_at_freebsd.org>> wrote:
> 
>     On 10/02/2016 23:28, Andriy Gapon wrote:
>     >
>     > Over a span of approximately 3 weeks I have got two slightly different panics of
>     > the same kind.   The affected system is a several months old amd64 head.
> 
>     I added a small assertion and it got tripped:
[snip]
> 
>     So, it seems that the code allows modification of read_ahead field of an entry
>     without holding a map lock.  I'd guess that that should be mostly harmless as
>     read_ahead value is not critical, but it is still not nice.
> 
> Not intentionally.  The nearby code to the read_ahead assignment expects the map
> to be locked, so I wouldn't categorize this as mostly harmless.
> 
> In general, you shouldn't get to the read_ahead assignment without the map being
> locked, and almost all of the code paths that unlock the map jump to RetryFault
> shortly thereafter, so it's hard to imagine how the map lock wouldn't be
> reacquired.  I speculate that the root cause of your panic is a case where
> vm_pager_get_pages() fails, and in such a case we might loop around, descending
> to the next backing object without reacquiring the map lock.  In other words,
> your above assertion failure is happening on the next iteration after an initial
> vm_pager_get_pages() failure, and we're about to do another vm_pager_get_pages()
> call on a different object.
> 
> In summary, I have no trouble believing that the code that handles a failure by
> vm_pager_get_pages() in vm_fault() is not quite right, and I think that's what's
> biting you.

Alan,

thank you very much for the very insightful analysis.
Indeed, I see that in this case the object chain is default -> swap -> swap.  I
am not sure how such chain was created.  It seems that going default -> swap is
not a problem as the map lock is not dropped in this case.  But going swap ->
swap the way you described (pager error, e.g. the page is just not found) has
exactly the problem that you suggested.

-- 
Andriy Gapon