Re: [PATCH v2 4/4] scsi: sym53c8xx_2: replace __get_free_pages() with kmalloc()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing `__get_free_pages()` with `kmalloc()` breaks the internal custom buddy allocator because `kmalloc()` does not guarantee page-aligned memory, leading to memory leaks, incorrect block coalescing, and potential kernel crashes.
--

commit c528e5943bc5d64b931842bf0abd4f0d0100f91e
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

scsi: sym53c8xx_2: replace __get_free_pages() with kmalloc()
    
This commit replaces __get_free_pages() and free_pages() with kmalloc() and
kfree() for the internal memory allocator of the sym53c8xx_2 driver. The goal
is to improve the API usage, as kfree() does not require knowing the size of
the freed object.
    
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/drivers/scsi/sym53c8xx_2/sym_hipd.h b/drivers/scsi/sym53c8xx_2/sym_hipd.h
> index 9231a28990643..aa365e8ba66fc 100644
> --- a/drivers/scsi/sym53c8xx_2/sym_hipd.h
> +++ b/drivers/scsi/sym53c8xx_2/sym_hipd.h
> @@ -1110,9 +1110,9 @@ sym_build_sge(struct sym_hcb *np, struct sym_tblmove *data, u64 badd, int len)
>   */
>  
>  #define sym_get_mem_cluster()	\
> -	(void *) __get_free_pages(GFP_ATOMIC, SYM_MEM_PAGE_ORDER)
> +	kmalloc(PAGE_SIZE << SYM_MEM_PAGE_ORDER, GFP_ATOMIC)

[Severity: High]
Will this break the custom buddy allocator logic in ___sym_mfree?

The custom buddy allocator relies on XOR arithmetic and bitwise AND to find and
coalesce buddy blocks, which fundamentally requires the base address of the root
block (the memory cluster) to be naturally aligned to its size:

drivers/scsi/sym53c8xx_2/sym_malloc.c:___sym_mfree() {
...
		b = a ^ s;
		q = &h[i];
		while (q->next && q->next != (m_link_p) b) {
			q = q->next;
		}
...
		q->next = q->next->next;
		a = a & b;
...
}

While __get_free_pages guarantees page alignment, kmalloc does not (especially
when KASAN or SLUB debugging is enabled).

If kmalloc returns an unaligned pointer, the XOR buddy calculation computes
incorrect buddy addresses. This prevents blocks from coalescing properly and
causes a memory leak.

Additionally, if blocks from differently offset clusters happen to merge, the
allocator reconstructs a naturally aligned base address that differs from the
original pointer returned by kmalloc. This can lead to calling kfree on an
invalid pointer, resulting in a kernel crash.

>  #define sym_free_mem_cluster(p)	\
> -	free_pages((unsigned long)p, SYM_MEM_PAGE_ORDER)
> +	kfree(p)
>  
>  /*
>   *  Link between free memory chunks of a given size.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260704-b4-scsi-v2-0-7d2d21a810de@kernel.org?part=4