The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Hao Li <hao.li@linux.dev>
To: Pengpeng Hou <pengpeng@iscas.ac.cn>
Cc: Vlastimil Babka <vbabka@kernel.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Harry Yoo <harry@kernel.org>,
	 Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	David Hildenbrand <david@kernel.org>,
	 Lorenzo Stoakes <ljs@kernel.org>,
	liam@infradead.org, Mike Rapoport <rppt@kernel.org>,
	 Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>, Jonathan Corbet <corbet@lwn.net>,
	 Shuah Khan <skhan@linuxfoundation.org>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] mm/slub: preserve previous object lifetime in user tracking
Date: Wed, 17 Jun 2026 15:54:37 +0800	[thread overview]
Message-ID: <ajJMzHe3W8a8NGqK@fedora> (raw)
In-Reply-To: <20260616141410.52117-3-pengpeng@iscas.ac.cn>

On Tue, Jun 16, 2026 at 10:14:08PM +0800, Pengpeng Hou wrote:
> SLAB_STORE_USER stores one allocation track and one free track for an
> object.  When that object is reused, the next allocation overwrites the
> allocation track.  If a stale pointer from the previous lifetime is later
> freed or otherwise reported, the free/check report can contain the victim
> allocation and the stale operation while the previous completed alloc/free
> pair has already been overwritten.
> 
> Keep one previous completed lifetime in the existing user tracking
> metadata.  When an object is allocated and the current allocation/free
> tracks both exist, copy that completed lifetime to the previous-lifetime
> slots before recording the new allocation.  Clear the current free track
> when the new allocation begins so the current lifetime does not continue
> to display a free from the old lifetime.
> 
> Print the previous object lifetime when it is available.  This is
> diagnostic information only; it does not infer semantic ownership or
> identify the root cause of a use-after-free.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  mm/slub.c | 66 +++++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 55 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 43d4febd5bf2..358f42e92207 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -327,7 +327,13 @@ struct track {
>  	unsigned long when;	/* When did the operation occur */
>  };
>  
> -enum track_item { TRACK_ALLOC, TRACK_FREE, TRACK_NR };
> +enum track_item {
> +	TRACK_ALLOC,
> +	TRACK_FREE,
> +	TRACK_PREV_ALLOC,
> +	TRACK_PREV_FREE,
> +	TRACK_NR,
> +};
>  
>  static inline unsigned int user_tracking_size(slab_flags_t flags)
>  {
> @@ -1080,12 +1086,37 @@ static void set_track_update(struct kmem_cache *s, void *object,
>  	p->when = jiffies;
>  }
>  
> -static __always_inline void set_track(struct kmem_cache *s, void *object,
> -				      enum track_item alloc, unsigned long addr, gfp_t gfp_flags)
> +static bool track_has_record(const struct track *t)
> +{
> +	return t->addr;
> +}

how about inline it

> +
> +static void clear_track(struct kmem_cache *s, void *object,
> +			enum track_item track)
> +{
> +	memset(get_track(s, object, track), 0, sizeof(struct track));
> +}
> +
> +static void save_previous_lifetime(struct kmem_cache *s, void *object)
> +{
> +	struct track *alloc = get_track(s, object, TRACK_ALLOC);
> +	struct track *free = get_track(s, object, TRACK_FREE);
> +
> +	if (!track_has_record(alloc) || !track_has_record(free))
> +		return;
> +
> +	*get_track(s, object, TRACK_PREV_ALLOC) = *alloc;
> +	*get_track(s, object, TRACK_PREV_FREE) = *free;

Maybe we can use memcpy instead of copying them one by one.

> +}
> +
> +static __always_inline void set_alloc_track(struct kmem_cache *s, void *object,
> +					    unsigned long addr, gfp_t gfp_flags)
>  {
>  	depot_stack_handle_t handle = set_track_prepare(gfp_flags);
>  
> -	set_track_update(s, object, alloc, addr, handle);
> +	save_previous_lifetime(s, object);
> +	set_track_update(s, object, TRACK_ALLOC, addr, handle);
> +	clear_track(s, object, TRACK_FREE);

sashiko has a comment:

https://sashiko.dev/#/patchset/20260616141410.52117-1-pengpeng%40iscas.ac.cn

It seems a simple fix could be removing clear_track() and allow the stale free
track.

>  }
>  
>  static void init_tracking(struct kmem_cache *s, void *object)
> @@ -1120,11 +1151,22 @@ static void print_track(const char *s, struct track *t, unsigned long pr_time)
>  void print_tracking(struct kmem_cache *s, void *object)
>  {
>  	unsigned long pr_time = jiffies;
> +	struct track *prev_alloc;
> +	struct track *prev_free;
> +
>  	if (!(s->flags & SLAB_STORE_USER))
>  		return;
>  
>  	print_track("Allocated", get_track(s, object, TRACK_ALLOC), pr_time);
>  	print_track("Freed", get_track(s, object, TRACK_FREE), pr_time);
> +
> +	prev_alloc = get_track(s, object, TRACK_PREV_ALLOC);
> +	prev_free = get_track(s, object, TRACK_PREV_FREE);
> +	if (track_has_record(prev_alloc) || track_has_record(prev_free)) {
> +		pr_err("Previous object lifetime:\n");
> +		print_track("Previously allocated", prev_alloc, pr_time);
> +		print_track("Previously freed", prev_free, pr_time);
> +	}
>  }
>  
>  static void print_slab_info(const struct slab *slab)
> @@ -1371,10 +1413,12 @@ check_bytes_and_report(struct kmem_cache *s, struct slab *slab,
>   *
>   * [Metadata starts at object + s->inuse]
>   *   - A. freelist pointer (if freeptr_outside_object)
> - *   - B. alloc tracking (SLAB_STORE_USER)
> - *   - C. free tracking (SLAB_STORE_USER)
> - *   - D. original request size (SLAB_KMALLOC && SLAB_STORE_USER)
> - *   - E. KASAN metadata (if enabled)
> + *   - B. current alloc tracking (SLAB_STORE_USER)
> + *   - C. current free tracking (SLAB_STORE_USER)
> + *   - D. previous alloc tracking (SLAB_STORE_USER)
> + *   - E. previous free tracking (SLAB_STORE_USER)
> + *   - F. original request size (SLAB_KMALLOC && SLAB_STORE_USER)
> + *   - G. KASAN metadata (if enabled)
>   *
>   * [Mandatory padding] (if CONFIG_SLUB_DEBUG && SLAB_RED_ZONE)
>   *   - One mandatory debug word to guarantee a minimum poisoned gap
> @@ -2029,8 +2073,8 @@ static inline void slab_pad_check(struct kmem_cache *s, struct slab *slab) {}
>  static inline int check_object(struct kmem_cache *s, struct slab *slab,
>  			void *object, u8 val) { return 1; }
>  static inline depot_stack_handle_t set_track_prepare(gfp_t gfp_flags) { return 0; }
> -static inline void set_track(struct kmem_cache *s, void *object,
> -			     enum track_item alloc, unsigned long addr, gfp_t gfp_flags) {}
> +static inline void set_alloc_track(struct kmem_cache *s, void *object,
> +				   unsigned long addr, gfp_t gfp_flags) {}
>  static inline void add_full(struct kmem_cache *s, struct kmem_cache_node *n,
>  					struct slab *slab) {}
>  static inline void remove_full(struct kmem_cache *s, struct kmem_cache_node *n,
> @@ -4522,7 +4566,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>  
>  success:
>  	if (kmem_cache_debug_flags(s, SLAB_STORE_USER))
> -		set_track(s, object, TRACK_ALLOC, addr, gfpflags);
> +		set_alloc_track(s, object, addr, gfpflags);
>  
>  	return object;
>  }
> -- 
> 2.43.0
> 
-- 
Thanks,
Hao

  reply	other threads:[~2026-06-17  7:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 14:14 [PATCH 0/4] mm/slub: preserve previous object lifetime Pengpeng Hou
2026-06-16 14:14 ` [PATCH 1/4] mm/slub: factor user tracking metadata size calculation Pengpeng Hou
2026-06-17  4:26   ` Hao Li
2026-06-16 14:14 ` [PATCH 2/4] mm/slub: preserve previous object lifetime in user tracking Pengpeng Hou
2026-06-17  7:54   ` Hao Li [this message]
2026-06-16 14:14 ` [PATCH 3/4] mm/slub: test previous lifetime tracking Pengpeng Hou
2026-06-16 14:14 ` [PATCH 4/4] Documentation/mm: document SLUB " Pengpeng Hou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajJMzHe3W8a8NGqK@fedora \
    --to=hao.li@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=harry@kernel.org \
    --cc=liam@infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=pengpeng@iscas.ac.cn \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rppt@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox