Re: [PATCH v3 07/13] s390/idals: Use kmalloc() for IDAL data buffers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] s390/idals: hardware alignment violation in IDAL buffers due to kmalloc()
--

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

s390/idals: Use kmalloc() for IDAL data buffers

idal_buffer_alloc() allocates the data chunks of an IDAL buffer that is
used for channel I/O. Replace use of __get_free_pages() with kmalloc()
and free_pages() with kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/arch/s390/include/asm/idals.h b/arch/s390/include/asm/idals.h
> index 06e1ec2afd5af..213f430a8d33a 100644
> --- a/arch/s390/include/asm/idals.h
> +++ b/arch/s390/include/asm/idals.h

[ ... ]

> @@ -135,21 +136,23 @@ struct idal_buffer {
>   * Allocate an idal buffer
>   */
>  static inline struct idal_buffer *idal_buffer_alloc(size_t size, int page_order)
>  {
>  	int nr_chunks, nr_ptrs, i;
>  	struct idal_buffer *ib;
>  	void *vaddr;
>  
>  	nr_ptrs = (size + IDA_BLOCK_SIZE - 1) >> IDA_SIZE_SHIFT;
>  	nr_chunks = (PAGE_SIZE << page_order) >> IDA_SIZE_SHIFT;
>  	ib = kmalloc_flex(*ib, data, nr_ptrs, GFP_DMA | GFP_KERNEL);
>  	if (!ib)
>  		return ERR_PTR(-ENOMEM);
>  	ib->size = size;
>  	ib->page_order = page_order;
>  	for (i = 0; i < nr_ptrs; i++) {
>  		if (i & (nr_chunks - 1)) {
>  			ib->data[i] = dma64_add(ib->data[i - 1], IDA_BLOCK_SIZE);
>  			continue;
>  		}
> -		vaddr = (void *)__get_free_pages(GFP_KERNEL, page_order);
> +		vaddr = kmalloc(PAGE_SIZE << page_order, GFP_KERNEL);

[Severity: Critical]
Does using kmalloc() here drop the hardware alignment guarantees required for
IDAL data buffers?

The s390 Channel Subsystem strictly requires all Indirect Data Address Words
(IDAWs), except the first, to be aligned to absolute 2K or 4K boundaries.

kmalloc() does not guarantee page alignment, especially when debugging options
like SLAB_RED_ZONE or KFENCE are enabled.

If an unaligned address is returned and passed to the channel subsystem, it
will generate a program check (I/O error).

Also, subsequent IDAW addresses are computed from the base pointer slightly
earlier in this loop:

	ib->data[i] = dma64_add(ib->data[i - 1], IDA_BLOCK_SIZE);

If the base pointer from kmalloc() is unaligned, does this corrupt all
subsequent IDAWs and violate the hardware alignment rules?

Furthermore, if the first IDAW is unaligned, the hardware transfers data only
up to the next 4K boundary (fewer than 4096 bytes).

The software size calculation earlier in idal_buffer_alloc() assumes exactly
4096 bytes are transferred per IDAW:

	nr_ptrs = (size + IDA_BLOCK_SIZE - 1) >> IDA_SIZE_SHIFT;

To satisfy the total transfer count, will the hardware attempt to fetch more
IDAWs than were allocated in the ib->data array?

This could lead to an out-of-bounds read where the hardware interprets random
data past the array bounds as a physical address, causing silent memory
corruption during DMA I/O.

>  		if (!vaddr)
>  			goto error;
> +		/* Only DMA addresses are retained in ib->data. */
> +		kmemleak_ignore(vaddr);
>  		ib->data[i] = virt_to_dma64(vaddr);
>  	}
>  	return ib;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-s390-cio-ready-v3-0-39e4894637d6@kernel.org?part=7