Re: [PATCH] powerpc/xive/spapr: correct bitmap allocation size

["Cédric Le Goater" <clg@kaod.org>]

On 6/23/22 20:25, Nathan Lynch wrote:
> kasan detects access beyond the end of the xibm->bitmap allocation:
> 
> BUG: KASAN: slab-out-of-bounds in _find_first_zero_bit+0x40/0x140
> Read of size 8 at addr c00000001d1d0118 by task swapper/0/1
> 
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.19.0-rc2-00001-g90df023b36dd #28
> Call Trace:
> [c00000001d98f770] [c0000000012baab8] dump_stack_lvl+0xac/0x108 (unreliable)
> [c00000001d98f7b0] [c00000000068faac] print_report+0x37c/0x710
> [c00000001d98f880] [c0000000006902c0] kasan_report+0x110/0x354
> [c00000001d98f950] [c000000000692324] __asan_load8+0xa4/0xe0
> [c00000001d98f970] [c0000000011c6ed0] _find_first_zero_bit+0x40/0x140
> [c00000001d98f9b0] [c0000000000dbfbc] xive_spapr_get_ipi+0xcc/0x260
> [c00000001d98fa70] [c0000000000d6d28] xive_setup_cpu_ipi+0x1e8/0x450
> [c00000001d98fb30] [c000000004032a20] pSeries_smp_probe+0x5c/0x118
> [c00000001d98fb60] [c000000004018b44] smp_prepare_cpus+0x944/0x9ac
> [c00000001d98fc90] [c000000004009f9c] kernel_init_freeable+0x2d4/0x640
> [c00000001d98fd90] [c0000000000131e8] kernel_init+0x28/0x1d0
> [c00000001d98fe10] [c00000000000cd54] ret_from_kernel_thread+0x5c/0x64
> 
> Allocated by task 0:
>   kasan_save_stack+0x34/0x70
>   __kasan_kmalloc+0xb4/0xf0
>   __kmalloc+0x268/0x540
>   xive_spapr_init+0x4d0/0x77c
>   pseries_init_irq+0x40/0x27c
>   init_IRQ+0x44/0x84
>   start_kernel+0x2a4/0x538
>   start_here_common+0x1c/0x20
> 
> The buggy address belongs to the object at c00000001d1d0118
>   which belongs to the cache kmalloc-8 of size 8
> The buggy address is located 0 bytes inside of
>   8-byte region [c00000001d1d0118, c00000001d1d0120)
> 
> The buggy address belongs to the physical page:
> page:c00c000000074740 refcount:1 mapcount:0 mapping:0000000000000000 index:0xc00000001d1d0558 pfn:0x1d1d
> flags: 0x7ffff000000200(slab|node=0|zone=0|lastcpupid=0x7ffff)
> raw: 007ffff000000200 c00000001d0003c8 c00000001d0003c8 c00000001d010480
> raw: c00000001d1d0558 0000000001e1000a 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>   c00000001d1d0000: fc 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>   c00000001d1d0080: fc fc 00 fc fc fc fc fc fc fc fc fc fc fc fc fc
>> c00000001d1d0100: fc fc fc 02 fc fc fc fc fc fc fc fc fc fc fc fc
>                              ^
>   c00000001d1d0180: fc fc fc fc 04 fc fc fc fc fc fc fc fc fc fc fc
>   c00000001d1d0200: fc fc fc fc fc 04 fc fc fc fc fc fc fc fc fc fc
> 
> This happens because the allocation uses the wrong unit (bits) when it
> should pass (BITS_TO_LONGS(count) * sizeof(long)) or equivalent. With small
> numbers of bits, the allocated object can be smaller than sizeof(long),
> which results in invalid accesses.
> 
> Use bitmap_zalloc() to allocate and initialize the irq bitmap, paired with
> bitmap_free() for consistency.
> 
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   arch/powerpc/sysdev/xive/spapr.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 7d5128676e83..d02911e78cfc 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -15,6 +15,7 @@
>   #include <linux/of_fdt.h>
>   #include <linux/slab.h>
>   #include <linux/spinlock.h>
> +#include <linux/bitmap.h>
>   #include <linux/cpumask.h>
>   #include <linux/mm.h>
>   #include <linux/delay.h>
> @@ -57,7 +58,7 @@ static int __init xive_irq_bitmap_add(int base, int count)
>   	spin_lock_init(&xibm->lock);
>   	xibm->base = base;
>   	xibm->count = count;
> -	xibm->bitmap = kzalloc(xibm->count, GFP_KERNEL);
> +	xibm->bitmap = bitmap_zalloc(xibm->count, GFP_KERNEL);
>   	if (!xibm->bitmap) {
>   		kfree(xibm);
>   		return -ENOMEM;
> @@ -75,7 +76,7 @@ static void xive_irq_bitmap_remove_all(void)
>   
>   	list_for_each_entry_safe(xibm, tmp, &xive_irq_bitmaps, list) {
>   		list_del(&xibm->list);
> -		kfree(xibm->bitmap);
> +		bitmap_free(xibm->bitmap);
>   		kfree(xibm);
>   	}
>   }