dma-buf: Fix silent overflow for phys vec to sgt

drivers/dma-buf/dma-buf-mapping.c line 16

[Severity: High]
This is a pre-existing issue, but splitting physical memory ranges using UINT_MAX can create unaligned scatterlist entries:

drivers/dma-buf/dma-buf-mapping.c:fill_sg_entry() {
	...
	for (i = 0; i < nents; i++) {
		len = min_t(size_t, length, UINT_MAX);
		...
		sg_dma_address(sgl) = addr + (dma_addr_t)i * UINT_MAX;
		...
	}
	...
}

Because UINT_MAX (0xFFFFFFFF) is not page-aligned, if a physical range exceeds UINT_MAX, the first segment has a non-page-aligned length, and the next segment will start at an unaligned DMA address.

Since IOMMU mapping and DMA engine APIs require middle segments to be aligned to hardware page boundaries, could passing unaligned addresses and sizes cause silent DMA corruption or explicit mapping failures? Would it be safer to align the max length chunk to the page mask (e.g., ALIGN_DOWN(UINT_MAX, PAGE_SIZE))?

[ ... ]

drivers/dma-buf/dma-buf-mapping.c static unsigned int calc_sg_nents(struct dma_iova_state *state, · line 45

[Severity: High]
This isn't a bug introduced by this patch, but on 32-bit architectures, size_t and unsigned int are 32 bits. The macro DIV_ROUND_UP(n, d) evaluates as ((n) + (d) - 1) / (d).

When phys_vec[i].len <= UINT_MAX (e.g., 4096), the expression 4096 + 0xFFFFFFFF - 1 wraps around in 32-bit unsigned arithmetic to 4094. Consequently, 4094 / 0xFFFFFFFF results in 0.

Does this cause calc_sg_nents() and fill_sg_entry() to compute 0 entries, forcing sg_alloc_table() to fail with -EINVAL and permanently breaking DMA mapping on 32-bit platforms?

drivers/dma-buf/dma-buf-mapping.c struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach, · line 146

[Severity: High]
This isn't a bug introduced by this patch, but if there's a mismatch between the caller-provided size and the sum of phys_vec[i].len, could it cause an out-of-bounds write?

drivers/dma-buf/dma-buf-mapping.c:dma_buf_phys_vec_to_sgt() {
	...
	if (dma->state && dma_use_iova(dma->state)) {
		WARN_ON_ONCE(mapped_len != size);
		ret = dma_iova_sync(attach->dev, dma->state, 0, mapped_len);
		if (ret)
			goto err_unmap_dma;

		sgl = fill_sg_entry(sgl, mapped_len, dma->state->addr);
	}
	...
}

If size is smaller than mapped_len, the SG table is allocated with nents derived from size, but fill_sg_entry() is later called with mapped_len. The loop inside fill_sg_entry() would then iterate more times than the allocated size of the scatterlist.

Could this cause sg_next() to return NULL, leading to a NULL pointer dereference and out-of-bounds write on the sgl object since execution proceeds past the WARN_ON_ONCE?

[Severity: High]
This is a pre-existing issue, but could the IOVA space allocated by dma_iova_try_alloc() be permanently leaked on multiple error paths?

drivers/dma-buf/dma-buf-mapping.c:dma_buf_phys_vec_to_sgt() {
	...
err_unmap_dma:
	if (!i || !dma->state) {
		; /* Do nothing */
	} else if (dma_use_iova(dma->state)) {
		dma_iova_destroy(attach->dev, dma->state, mapped_len, dir,
				 DMA_ATTR_MMIO);
	}
	...
err_free_state:
	kfree(dma->state);
err_free_dma:
	kfree(dma);
	return ERR_PTR(ret);
}

If sg_alloc_table() fails, execution jumps to err_free_state, which frees the state memory but doesn't call dma_iova_free() or dma_iova_destroy() to release the IOMMU address space.

Additionally, if dma_iova_link() fails on the first iteration (i == 0), it jumps to err_unmap_dma where (!i) is true, skipping dma_iova_destroy() and proceeding to err_free_state, again leaking the address space. Does this logic need adjusting to properly release the IOVA space?

[Severity: High]
This isn't a bug introduced by this patch, but dma_iova_link() appears to be consistently called with a hardcoded offset of 0:

drivers/dma-buf/dma-buf-mapping.c:dma_buf_phys_vec_to_sgt() {
	...
		} else if (dma_use_iova(dma->state)) {
			ret = dma_iova_link(attach->dev, dma->state,
					    phys_vec[i].paddr, 0,
					    phys_vec[i].len, dir,
					    DMA_ATTR_MMIO);
	...
}

If there are multiple physical ranges (nr_ranges > 1), wouldn't every single range be mapped to the exact same start address within the allocated IOVA space? Could this overwrite previous mappings and corrupt the IOMMU page tables? Should the offset parameter be the accumulated mapped_len instead of 0?

[Severity: High]
This isn't a bug introduced by this patch, but could the error recovery path improperly unmap scatterlist entries?

drivers/dma-buf/dma-buf-mapping.c:dma_buf_phys_vec_to_sgt() {
	...
err_unmap_dma:
	...
	} else {
		for_each_sgtable_dma_sg(&dma->sgt, sgl, i)
			dma_unmap_phys(attach->dev, sg_dma_address(sgl),
				       sg_dma_len(sgl), dir, DMA_ATTR_MMIO);
	}
	...
}

If dma_map_phys() fails mid-loop, for_each_sgtable_dma_sg() iterates over all dma->sgt.nents allocated entries. Unpopulated trailing entries will have addr=0 and len=0, leading to invalid dma_unmap_phys() calls.

Furthermore, if a range exceeded UINT_MAX, it was mapped via dma_map_phys() as a single contiguous chunk but split into multiple SG entries by fill_sg_entry(). Doesn't unmapping these sub-chunks piece-wise in the SG iteration loop violate the DMA API, which requires the exact contiguous dma_addr_t and size originally mapped?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1