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

[Andrew Morton <akpm@linux-foundation.org>]

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.