Re: [PATCH 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 driver's custom buddy allocator due to lost page-alignment guarantees, leading to memory leaks, free-list corruption, and kernel panics on `kfree()`.
--

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

scsi: sym53c8xx_2: replace __get_free_pages() with kmalloc()

This commit replaces __get_free_pages() with kmalloc() and free_pages() with
kfree() for the internal memory allocator in the sym53c8xx_2 driver.

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]
Does this change break the driver's custom buddy allocator?

The custom allocator in drivers/scsi/sym53c8xx_2/sym_malloc.c relies strictly
on the page-alignment guarantees provided by __get_free_pages().

When ___sym_mfree() is called, it computes buddy addresses using bitwise
operations:

drivers/scsi/sym53c8xx_2/sym_malloc.c:___sym_mfree() {
    ...
		b = a ^ s;
    ...
		a = a & b;
    ...
			M_FREE_MEM_CLUSTER((void *)a);
    ...
}

If kmalloc() returns an unaligned pointer (which can happen when
CONFIG_SLUB_DEBUG or KASAN is enabled), the XOR computation will yield an
incorrect buddy address.

Furthermore, the bitwise AND rounds the address down, losing any unaligned
offset provided by kmalloc(). When this rounded address is later passed to
kfree(), it can trigger a SLUB BUG or kernel panic because the pointer is
invalid.

Should this allocation remain page-aligned to satisfy the buddy allocator's
requirements?

>  #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/20260630-b4-scsi-v1-0-494fb37ebe7b@kernel.org?part=4