[PATCH v2] percpu: add basic double free check

[PATCH v2] percpu: add basic double free check

[Dennis Zhou]

This adds a basic double free check by validating the first bit of the
allocation in alloc_map and bound_map are set. If the alloc_map bit is
not set, then this means the area is currently unallocated. If the
bound_map bit is not set, then we are not freeing from the beginning of
the allocation.

This is a respin of [1] adding the requested changes from me and
Christoph.

[1] https://lore.kernel.org/linux-mm/20250904143514.Yk6Ap-jy@linutronix.de/

Signed-off-by: Dennis Zhou <dennis@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v2:
- moved pcpu_stats_area_dealloc()
- added additional check for bit_off out of bounds

 mm/percpu.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/mm/percpu.c b/mm/percpu.c
index 81462ce5866e..2f1ac2059a15 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -79,6 +79,7 @@
 #include <linux/mutex.h>
 #include <linux/percpu.h>
 #include <linux/pfn.h>
+#include <linux/ratelimit.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/vmalloc.h>
@@ -1276,18 +1277,24 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
 static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
 {
 	struct pcpu_block_md *chunk_md = &chunk->chunk_md;
+	int region_bits = pcpu_chunk_map_bits(chunk);
 	int bit_off, bits, end, oslot, freed;
 
 	lockdep_assert_held(&pcpu_lock);
-	pcpu_stats_area_dealloc(chunk);
 
 	oslot = pcpu_chunk_slot(chunk);
 
 	bit_off = off / PCPU_MIN_ALLOC_SIZE;
+	if (unlikely(bit_off < 0 || bit_off >= region_bits))
+		return 0;
+
+	/* check double free */
+	if (!test_bit(bit_off, chunk->alloc_map) ||
+	    !test_bit(bit_off, chunk->bound_map))
+		return 0;
 
 	/* find end index */
-	end = find_next_bit(chunk->bound_map, pcpu_chunk_map_bits(chunk),
-			    bit_off + 1);
+	end = find_next_bit(chunk->bound_map, region_bits, bit_off + 1);
 	bits = end - bit_off;
 	bitmap_clear(chunk->alloc_map, bit_off, bits);
 
@@ -1303,6 +1310,8 @@ static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
 
 	pcpu_chunk_relocate(chunk, oslot);
 
+	pcpu_stats_area_dealloc(chunk);
+
 	return freed;
 }
 
@@ -2225,6 +2234,7 @@ static void pcpu_balance_workfn(struct work_struct *work)
  */
 void free_percpu(void __percpu *ptr)
 {
+	static DEFINE_RATELIMIT_STATE(_rs, 60 * HZ, DEFAULT_RATELIMIT_BURST);
 	void *addr;
 	struct pcpu_chunk *chunk;
 	unsigned long flags;
@@ -2242,6 +2252,13 @@ void free_percpu(void __percpu *ptr)
 
 	spin_lock_irqsave(&pcpu_lock, flags);
 	size = pcpu_free_area(chunk, off);
+	if (size == 0) {
+		spin_unlock_irqrestore(&pcpu_lock, flags);
+
+		if (__ratelimit(&_rs))
+			WARN(1, "percpu double free or bad ptr\n");
+		return;
+	}
 
 	pcpu_alloc_tag_free_hook(chunk, off, size);
 
-- 
2.43.0

Re: [PATCH v2] percpu: add basic double free check

[Andrew Morton]

On Thu, 15 Jan 2026 18:32:16 -0800 Dennis Zhou <dennis@kernel.org> wrote:

> This adds a basic double free check by validating the first bit of the
> allocation in alloc_map and bound_map are set. If the alloc_map bit is
> not set, then this means the area is currently unallocated. If the
> bound_map bit is not set, then we are not freeing from the beginning of
> the allocation.
> 
> This is a respin of [1] adding the requested changes from me and
> Christoph.
> 
> ...
>
> @@ -1276,18 +1277,24 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
>  static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
>  {
>  	struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> +	int region_bits = pcpu_chunk_map_bits(chunk);
>  	int bit_off, bits, end, oslot, freed;
>  
>  	lockdep_assert_held(&pcpu_lock);
> -	pcpu_stats_area_dealloc(chunk);
>  
>  	oslot = pcpu_chunk_slot(chunk);
>  
>  	bit_off = off / PCPU_MIN_ALLOC_SIZE;
> +	if (unlikely(bit_off < 0 || bit_off >= region_bits))
> +		return 0;

This (which looks sensible) wasn't changelogged?

> @@ -2242,6 +2252,13 @@ void free_percpu(void __percpu *ptr)
>  
>  	spin_lock_irqsave(&pcpu_lock, flags);
>  	size = pcpu_free_area(chunk, off);
> +	if (size == 0) {
> +		spin_unlock_irqrestore(&pcpu_lock, flags);
> +
> +		if (__ratelimit(&_rs))
> +			WARN(1, "percpu double free or bad ptr\n");

Is ratelimiting really needed?  A WARN_ON_ONCE is enough to tell people
that this kernel is wrecked?

> +		return;
> +	}

The patch does appear to do that which it set out to do.  But do we
want to do it?  Is there a history of callers double-freeing percpu
memory?  Was there some bug which would have been more rapidly and
easily solved had this change been in place?

Re: [PATCH v2] percpu: add basic double free check

[Dennis Zhou]

On Fri, Jan 16, 2026 at 07:15:48PM -0800, Andrew Morton wrote:
> On Thu, 15 Jan 2026 18:32:16 -0800 Dennis Zhou <dennis@kernel.org> wrote:
> 
> > This adds a basic double free check by validating the first bit of the
> > allocation in alloc_map and bound_map are set. If the alloc_map bit is
> > not set, then this means the area is currently unallocated. If the
> > bound_map bit is not set, then we are not freeing from the beginning of
> > the allocation.
> > 
> > This is a respin of [1] adding the requested changes from me and
> > Christoph.
> > 
> > ...
> >
> > @@ -1276,18 +1277,24 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
> >  static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
> >  {
> >  	struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> > +	int region_bits = pcpu_chunk_map_bits(chunk);
> >  	int bit_off, bits, end, oslot, freed;
> >  
> >  	lockdep_assert_held(&pcpu_lock);
> > -	pcpu_stats_area_dealloc(chunk);
> >  
> >  	oslot = pcpu_chunk_slot(chunk);
> >  
> >  	bit_off = off / PCPU_MIN_ALLOC_SIZE;
> > +	if (unlikely(bit_off < 0 || bit_off >= region_bits))
> > +		return 0;
> 
> This (which looks sensible) wasn't changelogged?
> 

Sorry that's my fault. I can respin and add it if you'd like.

> > @@ -2242,6 +2252,13 @@ void free_percpu(void __percpu *ptr)
> >  
> >  	spin_lock_irqsave(&pcpu_lock, flags);
> >  	size = pcpu_free_area(chunk, off);
> > +	if (size == 0) {
> > +		spin_unlock_irqrestore(&pcpu_lock, flags);
> > +
> > +		if (__ratelimit(&_rs))
> > +			WARN(1, "percpu double free or bad ptr\n");
> 
> Is ratelimiting really needed?  A WARN_ON_ONCE is enough to tell people
> that this kernel is wrecked?
> 

I can see running multiple tests that might give me additional debug /
signal to how badly I screwed up. In production a WARN_ON_ONCE is
definitely more than enough, but might as well offer the chance to try
and trigger it more than once.

> > +		return;
> > +	}
> 
> The patch does appear to do that which it set out to do.  But do we
> want to do it?  Is there a history of callers double-freeing percpu
> memory?  Was there some bug which would have been more rapidly and
> easily solved had this change been in place?
> 

Originally, Sebastian posted he ran into the issue where he double freed
in [1] (linked in patch). Maybe he can elaborate how that bug was
introduced.

Wrt do we want to do it - I think it doesn't hurt and makes it more
explicit that something very wrong occurred. Percpu memory really 
expects users to be good samaritans. If you do happen to accidentally
double free without the warning, in a contrived case you could
experience unexplained behavior for some time before crashing in a spot
that would leave your head scratching. If anything I think there could
be an argument to fail louder.

[1] https://lore.kernel.org/linux-mm/20250904143514.Yk6Ap-jy@linutronix.de/

Thanks,
Dennis

Re: [PATCH v2] percpu: add basic double free check

[Andrew Morton]

On Fri, 16 Jan 2026 21:15:33 -0800 Dennis Zhou <dennis@kernel.org> wrote:

> On Fri, Jan 16, 2026 at 07:15:48PM -0800, Andrew Morton wrote:
> > On Thu, 15 Jan 2026 18:32:16 -0800 Dennis Zhou <dennis@kernel.org> wrote:
> > 
> > > This adds a basic double free check by validating the first bit of the
> > > allocation in alloc_map and bound_map are set. If the alloc_map bit is
> > > not set, then this means the area is currently unallocated. If the
> > > bound_map bit is not set, then we are not freeing from the beginning of
> > > the allocation.
> > > 
> > > This is a respin of [1] adding the requested changes from me and
> > > Christoph.
> > > 
> > > ...
> > >
> > > @@ -1276,18 +1277,24 @@ static int pcpu_alloc_area(struct pcpu_chunk *chunk, int alloc_bits,
> > >  static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
> > >  {
> > >  	struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> > > +	int region_bits = pcpu_chunk_map_bits(chunk);
> > >  	int bit_off, bits, end, oslot, freed;
> > >  
> > >  	lockdep_assert_held(&pcpu_lock);
> > > -	pcpu_stats_area_dealloc(chunk);
> > >  
> > >  	oslot = pcpu_chunk_slot(chunk);
> > >  
> > >  	bit_off = off / PCPU_MIN_ALLOC_SIZE;
> > > +	if (unlikely(bit_off < 0 || bit_off >= region_bits))
> > > +		return 0;
> > 
> > This (which looks sensible) wasn't changelogged?
> > 
> 
> Sorry that's my fault. I can respin and add it if you'd like.

Yes please, I'm thinking a respin is needed anyway....

> > > @@ -2242,6 +2252,13 @@ void free_percpu(void __percpu *ptr)
> > >  
> > >  	spin_lock_irqsave(&pcpu_lock, flags);
> > >  	size = pcpu_free_area(chunk, off);
> > > +	if (size == 0) {
> > > +		spin_unlock_irqrestore(&pcpu_lock, flags);
> > > +
> > > +		if (__ratelimit(&_rs))
> > > +			WARN(1, "percpu double free or bad ptr\n");
> > 
> > Is ratelimiting really needed?  A WARN_ON_ONCE is enough to tell people
> > that this kernel is wrecked?
> > 
> 
> I can see running multiple tests that might give me additional debug /
> signal to how badly I screwed up. In production a WARN_ON_ONCE is
> definitely more than enough, but might as well offer the chance to try
> and trigger it more than once.

If this is happening at development-time then developer can turn that
into WARN_ON() or whatever.

I dunno, I do feel that WARN_ON_ONCE is sufficient but no strong
feelings.

> > > +		return;
> > > +	}
> > 
> > The patch does appear to do that which it set out to do.  But do we
> > want to do it?  Is there a history of callers double-freeing percpu
> > memory?  Was there some bug which would have been more rapidly and
> > easily solved had this change been in place?
> > 
> 
> Originally, Sebastian posted he ran into the issue where he double freed
> in [1] (linked in patch). Maybe he can elaborate how that bug was
> introduced.
> 
> Wrt do we want to do it - I think it doesn't hurt and makes it more
> explicit that something very wrong occurred. Percpu memory really 
> expects users to be good samaritans. If you do happen to accidentally
> double free without the warning, in a contrived case you could
> experience unexplained behavior for some time before crashing in a spot
> that would leave your head scratching. If anything I think there could
> be an argument to fail louder.
> 
> [1] https://lore.kernel.org/linux-mm/20250904143514.Yk6Ap-jy@linutronix.de/

Could you please get this justification into the changelog?  It's
pretty important - explain to the world why we feel that Linux needs
alteration and what benefit we're providing.

Re: [PATCH v2] percpu: add basic double free check

[Sebastian Andrzej Siewior]

On 2026-01-16 21:15:33 [-0800], Dennis Zhou wrote:
> > The patch does appear to do that which it set out to do.  But do we
> > want to do it?  Is there a history of callers double-freeing percpu
> > memory?  Was there some bug which would have been more rapidly and
> > easily solved had this change been in place?
> > 
> 
> Originally, Sebastian posted he ran into the issue where he double freed
> in [1] (linked in patch). Maybe he can elaborate how that bug was
> introduced.
> 
> Wrt do we want to do it - I think it doesn't hurt and makes it more
> explicit that something very wrong occurred. Percpu memory really 
> expects users to be good samaritans. If you do happen to accidentally
> double free without the warning, in a contrived case you could
> experience unexplained behavior for some time before crashing in a spot
> that would leave your head scratching. If anything I think there could
> be an argument to fail louder.

I did write some code and there was a free path I did not expect to
happen. While it was rare to happen, it did not always crash right away.
Once I had the pattern I was wondering why none of the memory debug
switches did something. 
Adding this does not look expensive, allocating per-CPU is hardly done
in a hot path so I did not add anything but I don't mind hiding it
behind a config switch similar to SLAB_DEBUG.
While we do have way less per-CPU allocations compared to SLAB, I think
it is important to have some kind of sanity checks for it to ensure it
is used properly.
I would go with a WARN_ON_ONCE but if there is a desire for rate limited
multiple warnings, fine.

> [1] https://lore.kernel.org/linux-mm/20250904143514.Yk6Ap-jy@linutronix.de/
> 
> Thanks,
> Dennis

Sebastian