mm/slub: Fix incorrect interpretation of s->offset
Subsystems (2)
PATCH v2 1/1 mm/slub: Fix incorrect interpretation of s->offset
In a couple of places in the slub memory allocator, the code uses "s->offset" as a check to see if the free pointer is put right after the object. That check is no longer true with commit 3202fa62fb43 ("slub:
relocate freelist pointer to middle of object").
As a result, echoing "1" into the validate sysfs file, e.g. of dentry, may cause a bunch of "Freepointer corrupt" error reports like the following to appear with the system in panic afterwards.
[ 38.579769] ============================================================================= [ 38.580845] BUG dentry(666:pmcd.service) (Tainted: G B): Freepointer corrupt [ 38.581948] -----------------------------------------------------------------------------
To fix it, use the check "s->offset == s->inuse" in the new helper function freeptr_after_object() instead. Also add another helper function get_info_end() to return the end of info block (inuse + free pointer if not overlapping with object).
--- mm/slub.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)
mm/slub.c +22 -15
@@ -551,15 +551,29 @@ static void print_section(char *level, char *text, u8 *addr,metadata_access_disable (); } +static inline bool freeptr_after_object (struct kmem_cache *s) +{ +return s->offset == s->inuse; +} + +/* +* Return offset of the end of info block which is inuse + free pointer if +* not overlapping with object. +*/ +static inline unsigned int get_info_end (struct kmem_cache *s) +{ +if (freeptr_after_object (s)) +return s->inuse +sizeof (void *); +else +return s->inuse; +} +static struct track *get_track (struct kmem_cache *s,void *object,enum track_item alloc) {struct track *p; -if (s->offset) - p = object + s->offset +sizeof (void *); -else - p = object + s->inuse; + p = object +get_info_end (s);return p + alloc; }
@@ -693,10 +707,7 @@ static void print_trailer(struct kmem_cache *s, struct page *page, u8 *p)print_section (KERN_ERR ,"Redzone " , p + s->object_size, s->inuse - s->object_size); -if (s->offset) - off = s->offset +sizeof (void *); -else - off = s->inuse; + off =get_info_end (s);if (s->flags &SLAB_STORE_USER ) off +=2 *sizeof (struct track );
@@ -790,7 +801,7 @@ static int check_bytes_and_report(struct kmem_cache *s, struct page *page,* object address * Bytes of the object to be managed. * If the freepointer may overlay the object then the free -* pointer is the first word of the object. +* pointer is at the middle of the object. * * Poisoning uses 0x6b (POISON_FREE) and the last byte is * 0xa5 (POISON_END)
@@ -824,11 +835,7 @@ static int check_bytes_and_report(struct kmem_cache *s, struct page *page,static int check_pad_bytes (struct kmem_cache *s,struct page *page,u8 *p) { -unsigned long off = s->inuse;/* The end of info */ - -if (s->offset) -/* Freepointer is placed after the object. */ - off +=sizeof (void *); +unsigned long off =get_info_end (s);/* The end of info */ if (s->flags &SLAB_STORE_USER )/* We also have user information there */
@@ -915,7 +922,7 @@ static int check_object(struct kmem_cache *s, struct page *page,check_pad_bytes (s, page, p); } -if (!s->offset && val ==SLUB_RED_ACTIVE ) +if (!freeptr_after_object (s) && val ==SLUB_RED_ACTIVE )/* * Object and freepointer overlap. Cannot check * freepointer while object is allocated.
Signature · 1 line
2.18.1
Quoted 13 lines · Waiman Long
In a couple of places in the slub memory allocator, the code uses "
s->offset" as a check to see if the free pointer is put right after the object. That check is no longer true with commit3202fa62fb43("slub:
relocate freelist pointer to middle of object").As a result, echoing "1" into the validate sysfs file, e.g. of dentry, may cause a bunch of "Freepointer corrupt" error reports like the following to appear with the system in panic afterwards.
[ 38.579769] ============================================================================= [ 38.580845] BUG dentry(666:pmcd.service) (Tainted: G B): Freepointer corrupt [ 38.581948] -----------------------------------------------------------------------------
Quoted 7 lines
To fix it, use the check "
s->offset==s->inuse" in the new helper functionfreeptr_after_object()instead. Also add another helper functionget_info_end()to return the end of info block (inuse + free pointer if not overlapping with object).Fixes 3202fa62fb43 ("slub: relocate freelist pointer to middle of object")Signed-off-by Waiman Long <[email protected]>
Quoted 17 lines · Waiman Long
To fix it, use the check "
s->offset==s->inuse" in the new helper functionfreeptr_after_object()instead. Also add another helper functionget_info_end()to return the end of info block (inuse + free pointer if not overlapping with object).Fixes 3202fa62fb43 ("slub: relocate freelist pointer to middle of object")Signed-off-by Waiman Long <[email protected]>--- mm/slub.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)mm/slub.c +0
-0
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 554
Quoted 4 lines
mm/slub.c +1
-0@@ -551,15 +551,29 @@ static void print_section(char *level, char *text, u8 *addr,metadata_access_disable (); } +static inline bool freeptr_after_object (struct kmem_cache *s)
bool freeptr_outside_of_object()?
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 556
Quoted 2 lines
mm/slub.c +2
-0+{ +return s->offset == s->inuse;
s->offset >= s->inuse?
There may be a redzone after the object.
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 563
Quoted 1 line
mm/slub.c +1
-0+static inline unsigned int get_info_end (struct kmem_cache *s)
static inline track_offset()?
Quoted 21 lines · Christopher Lameter
Quoted 19 lines · Waiman Long
To fix it, use the check "
s->offset==s->inuse" in the new helper functionfreeptr_after_object()instead. Also add another helper functionget_info_end()to return the end of info block (inuse + free pointer if not overlapping with object).Fixes 3202fa62fb43 ("slub: relocate freelist pointer to middle of object")Signed-off-by Waiman Long <[email protected]>--- mm/slub.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-)mm/slub.c +0
-0@@ -551,15 +551,29 @@ static void print_section(char *level, char *text, u8 *addr,metadata_access_disable (); }
Quoted 3 lines
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 554
Quoted 1 line
mm/slub.c +1
-0+static inline bool freeptr_after_object (struct kmem_cache *s)bool
freeptr_outside_of_object()?
I can change to that name. It doesn't really matter to me.
Quoted 6 lines
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 556
Quoted 2 lines
mm/slub.c +2
-0+{ +return s->offset == s->inuse;
s->offset>=s->inuse?There may be a redzone after the object.
Technically inuse is object + red zone. According to calculate_sizes():
s->inuse = size;
if (((flags & (SLAB_TYPESAFE_BY_RCU | SLAB_POISON)) || s->ctor)) {
/*
* Relocate free pointer after the object if it is not
* permitted to overwrite the first word of the object on
* kmem_cache_free.
*
* This is the case if we do RCU, have a constructor or
* destructor or are poisoning the objects. */
s->offset = size;
size += sizeof(void *);
So (s->offset == s->inuse) when the free pointer is outside of the object.
Quoted 3 lines
mm/slub.c static void print_section(char *level, char *text, u8 *addr, · line 563
Quoted 1 line
mm/slub.c +1
-0+static inline unsigned int get_info_end (struct kmem_cache *s)static inline
track_offset()?
The main reason why I don't use that is because there is a track data structure in slub. There are functions name get_track() and set_track(). I don't want to confuse with them.
Cheers,
Longman
Quoted 5 lines · Waiman Long
Quoted 4 lines
Quoted 2 lines
+static inline unsigned int
get_info_end(struct kmem_cache *s)static inline
track_offset()?The main reason why I don't use that is because there is a track data structure in slub. There are functions name
get_track()andset_track(). I don't want to confuse with them.
Oh, I now realize that Matt meant the offset of the track data structure. Depending on the debug option, there may not be a track data structure at that offset.
Cheers,
Longman