Re: [PATCH 4/4] mm/slub: refactor calculate_order() and calc_slab_order()

[Vlastimil Babka]

On 9/11/23 07:56, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed "UBSAN:shift-out-of-bounds_in_mm/slub.c" on:
> 
> commit: f04d441027621c16081803832a54f59272112cf5 ("[PATCH 4/4] mm/slub: refactor calculate_order() and calc_slab_order()")
> url: https://github.com/intel-lab-lkp/linux/commits/Vlastimil-Babka/mm-slub-simplify-the-last-resort-slab-order-calculation/20230908-225506
> base: git://git.kernel.org/cgit/linux/kernel/git/vbabka/slab.git for-next
> patch link: https://lore.kernel.org/all/20230908145302.30320-10-vbabka@xxxxxxx/
> patch subject: [PATCH 4/4] mm/slub: refactor calculate_order() and calc_slab_order()
> 
> in testcase: boot
> 
> compiler: clang-16
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> +-------------------------------------------------+------------+------------+
> |                                                 | a17847b835 | f04d441027 |
> +-------------------------------------------------+------------+------------+
> | UBSAN:shift-out-of-bounds_in_mm/slub.c          | 0          | 12         |
> +-------------------------------------------------+------------+------------+
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> | Closes: https://lore.kernel.org/oe-lkp/202309111340.f59c3f22-oliver.sang@xxxxxxxxx
> 
> 
> [    0.901457][    T0] UBSAN: shift-out-of-bounds in mm/slub.c:463:34
> [    0.902458][    T0] shift exponent 52 is too large for 32-bit type 'unsigned int'
> [    0.903477][    T0] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G                T  6.5.0-rc1-00009-gf04d44102762 #1
> [    0.904450][    T0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [    0.904450][    T0] Call Trace:
> [    0.904450][    T0]  <TASK>
> [ 0.904450][ T0] dump_stack_lvl (lib/dump_stack.c:107) 
> [ 0.904450][ T0] dump_stack (lib/dump_stack.c:114) 
> [ 0.904450][ T0] ubsan_epilogue (lib/ubsan.c:218) 
> [ 0.904450][ T0] __ubsan_handle_shift_out_of_bounds (lib/ubsan.c:?) 
> [ 0.904450][ T0] ? tdx_handle_virt_exception (arch/x86/include/asm/shared/tdx.h:60 arch/x86/coco/tdx/tdx.c:375 arch/x86/coco/tdx/tdx.c:430 arch/x86/coco/tdx/tdx.c:650 arch/x86/coco/tdx/tdx.c:666) 
> [ 0.904450][ T0] ? kmemleak_alloc (mm/kmemleak.c:977) 
> [ 0.904450][ T0] __kmem_cache_create (mm/slub.c:? mm/slub.c:4159 mm/slub.c:4473 mm/slub.c:4507 mm/slub.c:5104) 

Oh thanks, fixing up:

--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4152,7 +4152,8 @@ static inline int calculate_order(unsigned int size)
                        nr_cpus = nr_cpu_ids;
                min_objects = 4 * (fls(nr_cpus) + 1);
        }
-       max_objects = order_objects(slub_max_order, size);
+       /* min_objects can't be 0 because get_order(0) is undefined */
+       max_objects = max(order_objects(slub_max_order, size), 1);
        min_objects = min(min_objects, max_objects);
 
        min_order = max(slub_min_order, (unsigned int)get_order(min_objects * size));
l