[PATCH] mm/percpu: Add a simple double-free check for per-CPU memory

[PATCH] mm/percpu: Add a simple double-free check for per-CPU memory

[Sebastian Andrzej Siewior]

The free path clears the allocation bits in pcpu_chunk::alloc_map. A
simple double free check would be to check if the bits, which are about
to be cleared, are already cleared.

Check if the bit is already cleared. Issue a warning and abort free in
that case.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

I managed accidentally a double free recently. This would have noticed
it. It looks low overhead so there might be no need to hide it behind a
debug switch.

 mm/percpu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/percpu.c b/mm/percpu.c
index d9cbaee92b605..a2abddd85294a 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1276,7 +1276,7 @@ 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 bit_off, bits, end, oslot, freed;
+	int bit_off, bits, end, oslot, freed, free_bit;
 
 	lockdep_assert_held(&pcpu_lock);
 	pcpu_stats_area_dealloc(chunk);
@@ -1289,6 +1289,11 @@ static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
 	end = find_next_bit(chunk->bound_map, pcpu_chunk_map_bits(chunk),
 			    bit_off + 1);
 	bits = end - bit_off;
+
+	free_bit = find_next_bit(chunk->alloc_map, end, bit_off);
+	if (WARN(free_bit != bit_off, "Trying to free already free memory"))
+		return 0;
+
 	bitmap_clear(chunk->alloc_map, bit_off, bits);
 
 	freed = bits * PCPU_MIN_ALLOC_SIZE;
-- 
2.51.0

Re: [PATCH] mm/percpu: Add a simple double-free check for per-CPU memory

[Andrew Morton]

On Thu, 4 Sep 2025 16:35:14 +0200 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> The free path clears the allocation bits in pcpu_chunk::alloc_map. A
> simple double free check would be to check if the bits, which are about
> to be cleared, are already cleared.
> 

Nice idea.  It's hardly a fast path!

Re: [PATCH] mm/percpu: Add a simple double-free check for per-CPU memory

[Christoph Lameter (Ampere)]

On Thu, 4 Sep 2025, Andrew Morton wrote:

> On Thu, 4 Sep 2025 16:35:14 +0200 Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
> > The free path clears the allocation bits in pcpu_chunk::alloc_map. A
> > simple double free check would be to check if the bits, which are about
> > to be cleared, are already cleared.
> >
>
> Nice idea.  It's hardly a fast path!

And it could be improved by checking the bit instead of finding the next
one.

Re: [PATCH] mm/percpu: Add a simple double-free check for per-CPU memory

[Dennis Zhou]

Hello,

On Thu, Sep 04, 2025 at 04:35:14PM +0200, Sebastian Andrzej Siewior wrote:
> The free path clears the allocation bits in pcpu_chunk::alloc_map. A
> simple double free check would be to check if the bits, which are about
> to be cleared, are already cleared.
> 

I thought about the double free issue in the past. It's a bit imperfect
because the same pointer is handed out with no metadata for future
allocations. Worse, the whole percpu chunk could go away too should you
be so unlucky and then the whole path to get to the chunk goes awry.

> Check if the bit is already cleared. Issue a warning and abort free in
> that case.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> 
> I managed accidentally a double free recently. This would have noticed
> it. It looks low overhead so there might be no need to hide it behind a
> debug switch.
> 
>  mm/percpu.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/percpu.c b/mm/percpu.c
> index d9cbaee92b605..a2abddd85294a 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -1276,7 +1276,7 @@ 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 bit_off, bits, end, oslot, freed;
> +	int bit_off, bits, end, oslot, freed, free_bit;
>  
>  	lockdep_assert_held(&pcpu_lock);
>  	pcpu_stats_area_dealloc(chunk);
> @@ -1289,6 +1289,11 @@ static int pcpu_free_area(struct pcpu_chunk *chunk, int off)
>  	end = find_next_bit(chunk->bound_map, pcpu_chunk_map_bits(chunk),
>  			    bit_off + 1);
>  	bits = end - bit_off;
> +
> +	free_bit = find_next_bit(chunk->alloc_map, end, bit_off);
> +	if (WARN(free_bit != bit_off, "Trying to free already free memory"))
> +		return 0;
> +

We might want to test_bit(bit_off, chunk->bound_map) also to make sure
we aren't accidentally doing partial frees if the region becomes say the
second half of a larger allocation before the double free.

Possibly good to move the WARN to free_percpu() and exit early too for
the other hooks rather than do all that with free size of 0.

>  	bitmap_clear(chunk->alloc_map, bit_off, bits);
>  
>  	freed = bits * PCPU_MIN_ALLOC_SIZE;
> -- 
> 2.51.0
> 

Thanks,
Dennis