diff mbox series

powerpc/xive/spapr: correct bitmap allocation size

Message ID 20220623182509.3985625-1-nathanl@linux.ibm.com (mailing list archive)
State Accepted
Headers show
Series powerpc/xive/spapr: correct bitmap allocation size | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 10 jobs.
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 10 jobs.
snowpatch_ozlabs/github-powerpc_kernel_qemu success Successfully ran 23 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 7 jobs.

Commit Message

Nathan Lynch June 23, 2022, 6:25 p.m. UTC
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>
---
 arch/powerpc/sysdev/xive/spapr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Cédric Le Goater June 23, 2022, 6:40 p.m. UTC | #1
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);
>   	}
>   }
Michael Ellerman June 29, 2022, 7:01 a.m. UTC | #2
On Thu, 23 Jun 2022 13:25:09 -0500, 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
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/xive/spapr: correct bitmap allocation size
      https://git.kernel.org/powerpc/c/19fc5bb93c6bbdce8292b4d7eed04e2fa118d2fe

cheers
diff mbox series

Patch

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);
 	}
 }