On Thu, Aug 9, 2018 at 1:52 AM, Vlastimil Babka <[email protected]> wrote: > In SLUB, prefetch_freepointer() is used when allocating an object from cache's > freelist, to make sure the next object in the list is cache-hot, since it's > probable it will be allocated soon. > > Commit 2482ddec670f ("mm: add SLUB free list pointer obfuscation") has > unintentionally changed the prefetch in a way where the prefetch is turned to > a > real fetch, and only the next->next pointer is prefetched. In case there is > not > a stream of allocations that would benefit from prefetching, the extra real > fetch might add a useless cache miss to the allocation. Restore the previous > behavior. > > Signed-off-by: Vlastimil Babka <[email protected]> > Cc: Kees Cook <[email protected]> > Cc: Daniel Micay <[email protected]> > Cc: Eric Dumazet <[email protected]> > Cc: Christoph Lameter <[email protected]> > Cc: Pekka Enberg <[email protected]> > Cc: David Rientjes <[email protected]> > Cc: Joonsoo Kim <[email protected]> > --- > While I don't expect this to be causing the bug at hand, it's worth fixing. > For the bug it might mean that the page fault moves elsewhere. > > mm/slub.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 51258eff4178..ce2b9e5cea77 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -271,8 +271,7 @@ static inline void *get_freepointer(struct kmem_cache *s, > void *object) > > static void prefetch_freepointer(const struct kmem_cache *s, void *object) > { > - if (object) > - prefetch(freelist_dereference(s, object + s->offset)); > + prefetch(object + s->offset);
Ah -- gotcha. I think I misunderstood the purpose here. You're not prefetching what is being pointed at, you're literally prefetching what is stored there. That wouldn't require dereferencing the freelist pointer, no. Thanks! Acked-by: Kees Cook <[email protected]> > } > > static inline void *get_freepointer_safe(struct kmem_cache *s, void *object) > -- > 2.18.0 > -- Kees Cook Pixel Security
