Messages in this thread | | | Date | Tue, 11 Jul 2023 09:21:36 -0700 | From | Kees Cook <> | Subject | Re: [PATCH 2/2] mm/slub: remove freelist_dereference() |
| |
On Tue, Jul 11, 2023 at 03:46:25PM +0200, Vlastimil Babka wrote: > freelist_dereference() is a one-liner only used from get_freepointer(). > Remove it and make get_freepointer() call freelist_ptr_decode() > directly to make the code easier to follow. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/slub.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 07edad305512..c4556a5dab4b 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -397,18 +397,14 @@ static inline void *freelist_ptr_decode(const struct kmem_cache *s, > return decoded; > } > > -/* Returns the freelist pointer recorded at location ptr_addr. */ > -static inline void *freelist_dereference(const struct kmem_cache *s, > - void *ptr_addr) > -{ > - return freelist_ptr_decode(s, *(freeptr_t *)(ptr_addr), > - (unsigned long)ptr_addr); > -} > - > static inline void *get_freepointer(struct kmem_cache *s, void *object) > { > - object = kasan_reset_tag(object); > - return freelist_dereference(s, (freeptr_t *)(object + s->offset)); > + unsigned long ptr_addr; > + freeptr_t p; > + > + ptr_addr = ((unsigned long)kasan_reset_tag(object)) + s->offset; > + p = *(freeptr_t *)(ptr_addr); > + return freelist_ptr_decode(s, p, ptr_addr); > } > > #ifndef CONFIG_SLUB_TINY > -- > 2.41.0 >
I like reducing the complexity here, but I find dropping the "object" reassignment makes this a bit harder to read. What about:
object = kasan_reset_tag(object); unsigned long ptr_addr = (unsigned long)object + s->offset; freeptr_t p = *(freeptr_t *)(ptr_addr);
return freelist_ptr_decode(s, p, ptr_addr);
?
They're the same result, so either way:
Acked-by: Kees Cook <keescook@chromium.org>
-- Kees Cook
| |